From c08d9ec858b47c3a707fe3457da7da27314eff36 Mon Sep 17 00:00:00 2001 From: sseerrggii Date: Mon, 27 Feb 2017 17:44:24 +0100 Subject: [PATCH 01/58] Force 100% width to select2 jQuery --- app/assets/stylesheets/application.css.scss | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/app/assets/stylesheets/application.css.scss b/app/assets/stylesheets/application.css.scss index 473ae6835..4aa50eedf 100644 --- a/app/assets/stylesheets/application.css.scss +++ b/app/assets/stylesheets/application.css.scss @@ -394,6 +394,11 @@ label[required]::after{ margin: 0 auto; } +//fix select2 100px +.select2-container { + width: 100% !important; +} + .navbar-form { padding: 0; background-color: transparent; From 72b7bad68f343df58efb7b6879fe38faad3fb4f5 Mon Sep 17 00:00:00 2001 From: Pau Perez Date: Fri, 28 Jul 2017 19:37:20 +0200 Subject: [PATCH 02/58] Move #give_time from users to transfer controller This duplicates the UsersController#give_time to TransferController#new as it should be by Rails conventions. We aim to stick to REST conventions of the Transfer resource instead of having custom actions on users and organizations controllers. --- app/controllers/transfers_controller.rb | 37 ++++++++++ config/routes.rb | 2 +- spec/controllers/transfers_controller_spec.rb | 71 ++++++++++++++++++- 3 files changed, 108 insertions(+), 2 deletions(-) diff --git a/app/controllers/transfers_controller.rb b/app/controllers/transfers_controller.rb index 8d4f4f432..583baeece 100644 --- a/app/controllers/transfers_controller.rb +++ b/app/controllers/transfers_controller.rb @@ -14,6 +14,23 @@ def create redirect_to redirect_target end + def new + @user = scoped_users.find(params[:id]) + @destination = @user + .members + .find_by(organization: current_organization) + .account + .id + @source = find_transfer_source + @offer = find_transfer_offer + @transfer = Transfer.new( + source: @source, + destination: @destination, + post: @offer + ) + @sources = find_transfer_sources_for_admin + end + def delete_reason @transfer = Transfer.find(params[:id]) @transfer.update_columns(reason: nil) @@ -25,6 +42,26 @@ def delete_reason private + def find_transfer_sources_for_admin + return unless admin? + [current_organization.account] + + current_organization.member_accounts.where("members.active is true") + end + + def find_transfer_offer + current_organization.offers. + find(params[:offer]) if params[:offer].present? + end + + def find_transfer_source + current_user.members. + find_by(organization: current_organization).account.id + end + + def scoped_users + current_organization.users + end + def find_source if admin? Account.find(transfer_params[:source]) diff --git a/config/routes.rb b/config/routes.rb index e03841279..409c52190 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -31,7 +31,7 @@ resources :users, concerns: :accountable, except: :destroy, :path => "members" - resources :transfers, only: [:create] do + resources :transfers, only: [:create, :new] do member do put :delete_reason end diff --git a/spec/controllers/transfers_controller_spec.rb b/spec/controllers/transfers_controller_spec.rb index 1d8d6c908..e44614f49 100644 --- a/spec/controllers/transfers_controller_spec.rb +++ b/spec/controllers/transfers_controller_spec.rb @@ -5,12 +5,81 @@ 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) } + include_context "stub browser locale" + before { set_browser_locale('ca') } - describe "POST #create" do + describe '#new' do + let(:user) { member_giver.user } + let(:params) { { id: user.id } } + before { login(user) } + it 'renders the :new template' do + expect(get :new, params).to render_template(:new) + end + + it 'finds the user' do + get :new, params + expect(assigns(:user)).to eq(user) + end + + it 'finds the destination account' do + get :new, params + account = user.members.find_by(organization: user.organizations.first).account + + expect(assigns(:destination)).to eq(account.id) + end + + it 'finds the transfer source' do + get :new, params + source = user.members.find_by(organization: user.organizations.first).account.id + + expect(assigns(:source)).to eq(source) + end + + context 'when the offer is specified' do + let(:offer) { Fabricate(:offer, organization: user.organizations.first) } + + before { params.merge!(offer: offer.id) } + + it 'finds the transfer offer' do + get :new, params + offer = user.organizations.first.offers.find(params[:offer]) + + expect(assigns(:offer)).to eq(offer) + end + end + + context 'when the offer is not specified' do + it 'does not find any offer' do + get :new, params + expect(assigns(:offer)).to be_nil + end + end + + context 'when the user is admin of the current organization' do + let(:user) { member_admin.user } + + it 'finds all accounts in the organization as sources' do + get :new, params + expect(assigns(:sources)).to contain_exactly( + test_organization.account, member_admin.account + ) + end + end + + context 'when the user is not admin of the current organization' do + it 'does not assign :sources' do + get :new, params + expect(assigns(:sources)).to be_nil + end + end + end + + describe "POST #create" do + before { login(user) } context "with valid params" do context "with an admin user logged" do From 9a1d6b098af55a4da0f6d941c8be7d7773efe2e2 Mon Sep 17 00:00:00 2001 From: Pau Perez Date: Wed, 16 Aug 2017 19:04:26 +0200 Subject: [PATCH 03/58] Move #give_time from orgs to transfers controller This duplicates the OrganizationsController#give_time to TransferController#new as it should be by Rails conventions. We aim to stick to REST conventions of the Transfer resource instead of having custom actions on users and organizations controllers. --- app/controllers/transfers_controller.rb | 61 ++++++--- app/views/transfers/new.html.erb | 66 ++++++++++ app/views/transfers/new_organization.html.erb | 63 ++++++++++ spec/controllers/transfers_controller_spec.rb | 116 ++++++++++++------ 4 files changed, 250 insertions(+), 56 deletions(-) create mode 100644 app/views/transfers/new.html.erb create mode 100644 app/views/transfers/new_organization.html.erb diff --git a/app/controllers/transfers_controller.rb b/app/controllers/transfers_controller.rb index 583baeece..64b70eaf5 100644 --- a/app/controllers/transfers_controller.rb +++ b/app/controllers/transfers_controller.rb @@ -15,20 +15,32 @@ def create end def new - @user = scoped_users.find(params[:id]) - @destination = @user - .members - .find_by(organization: current_organization) - .account - .id - @source = find_transfer_source - @offer = find_transfer_offer - @transfer = Transfer.new( - source: @source, - destination: @destination, - post: @offer - ) - @sources = find_transfer_sources_for_admin + if for_organization? + load_resource + + @source = find_transfer_source + @offer = find_transfer_offer + @destination = @organization.account.id + @transfer = Transfer.new(source: @source, destination: @destination) + @sources = find_transfer_sources_for_admin + + render :new_organization + else + @user = scoped_users.find(params[:id]) + @destination = @user + .members + .find_by(organization: current_organization) + .account + .id + @source = find_transfer_source + @offer = find_transfer_offer + @transfer = Transfer.new( + source: @source, + destination: @destination, + post: @offer + ) + @sources = find_transfer_sources_for_admin + end end def delete_reason @@ -42,6 +54,18 @@ def delete_reason private + def for_organization? + params.key?('organization') + end + + def load_resource + if params[:id] + @organization = Organization.find(params[:id]) + else + @organizations = Organization.all + end + end + def find_transfer_sources_for_admin return unless admin? [current_organization.account] + @@ -54,8 +78,13 @@ def find_transfer_offer end def find_transfer_source - current_user.members. - find_by(organization: current_organization).account.id + if for_organization? + current_user.members. + find_by(organization: @organization).account.id + else + current_user.members. + find_by(organization: current_organization).account.id + end end def scoped_users diff --git a/app/views/transfers/new.html.erb b/app/views/transfers/new.html.erb new file mode 100644 index 000000000..2b5b143df --- /dev/null +++ b/app/views/transfers/new.html.erb @@ -0,0 +1,66 @@ +

+ + <%= t ".give_time" %> + + <%= link_to @user.member(current_organization).display_name_with_uid, user_path(@user) %> +

+<% if @offer %> +

+ <%= @offer %> +

+<% end %> +<%= simple_form_for @transfer do |f| %> +
+ <%= f.input :hours, + as: :integer, + input_html: { + min: 0, + max: 20, + "data-rule-either-hours-minutes-informed" => "true", + "data-rule-range" => "[0,20]" + } %> + <%= 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.input :destination, as: :hidden %> + <% if admin? %> +
+ <%= f.label :source %> + <%= f.select :source, + options_for_select( + @sources.sort_by { |source| [source.accountable_type, source.accountable.try(:member_uid)] } + .map { |a| ["#{a.accountable_type == "Member" ? a.accountable.member_uid : ""} #{a.accountable_type} #{a.accountable.to_s}", a.id] }, + selected: current_user.member(current_organization).account.id + ), + {}, + id: "select2-time", + class: "form-control" %> +
+ <% end %> + <% if @offer %> + <%= f.input :post, readonly: true %> + <%= f.input :post_id, as: :hidden %> + <% else %> + <%= f.input :post_id, collection: @user.member(current_organization).offers.active %> + <% end %> +
+
+ <%= f.button :submit, class: "btn btn-default", style: "margin-bottom: 20px;" %> +
+<% end %> + + diff --git a/app/views/transfers/new_organization.html.erb b/app/views/transfers/new_organization.html.erb new file mode 100644 index 000000000..02787bdef --- /dev/null +++ b/app/views/transfers/new_organization.html.erb @@ -0,0 +1,63 @@ +

+ <%= t '.give_time' %> + <%= link_to @organization, organization_path(@organization) %> +

+<% if @offer %> +

+ <%= @offer %> +

+<% end %> +<%= simple_form_for @transfer do |f| %> +
+ <%= f.input :hours, + as: :integer, + input_html: { + min: 0, + max: 20, + 'data-rule-either-hours-minutes-informed' => 'true', + 'data-rule-range' => '[0,20]' + } %> + <%= 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.input :destination, as: :hidden %> + <% if admin? %> +
+ <%= f.label :source %> + <%# TODO: refactor. it's very complex for a view. %> + <%= f.select :source, + options_for_select( + @sources.sort_by { |source| [source.accountable_type, source.accountable_id] } + .map { |a| ["#{a.accountable_type == 'Member' ? a.accountable.member_uid : a.accountable_id} #{a.accountable_type} #{a.accountable.to_s}", a.id] }, + selected: current_user.member(current_organization).account.id + ), + {}, + id: 'select2-time', + class: 'form-control' %> +
+ <% end %> + <% if @offer %> + <%= f.input :post, as: :hidden %> + <% end %> +
+
+ <%= f.button :submit, + class: 'btn btn-default', + style: 'margin-bottom: 20px;' %> +
+<% end %> + diff --git a/spec/controllers/transfers_controller_spec.rb b/spec/controllers/transfers_controller_spec.rb index e44614f49..ca3e93cc1 100644 --- a/spec/controllers/transfers_controller_spec.rb +++ b/spec/controllers/transfers_controller_spec.rb @@ -12,68 +12,104 @@ describe '#new' do let(:user) { member_giver.user } - let(:params) { { id: user.id } } before { login(user) } - it 'renders the :new template' do - expect(get :new, params).to render_template(:new) - end + context 'when the destination is a user account' do + let(:params) { { id: user.id } } - it 'finds the user' do - get :new, params - expect(assigns(:user)).to eq(user) - end + it 'renders the :new template' do + expect(get :new, params).to render_template(:new) + end - it 'finds the destination account' do - get :new, params - account = user.members.find_by(organization: user.organizations.first).account + it 'finds the user' do + get :new, params + expect(assigns(:user)).to eq(user) + end - expect(assigns(:destination)).to eq(account.id) - end + it 'finds the destination account' do + get :new, params + account = user.members.find_by(organization: user.organizations.first).account - it 'finds the transfer source' do - get :new, params - source = user.members.find_by(organization: user.organizations.first).account.id + expect(assigns(:destination)).to eq(account.id) + end - expect(assigns(:source)).to eq(source) - end + it 'finds the transfer source' do + get :new, params + source = user.members.find_by(organization: user.organizations.first).account.id - context 'when the offer is specified' do - let(:offer) { Fabricate(:offer, organization: user.organizations.first) } + expect(assigns(:source)).to eq(source) + end - before { params.merge!(offer: offer.id) } + context 'when the offer is specified' do + let(:offer) { Fabricate(:offer, organization: user.organizations.first) } - it 'finds the transfer offer' do - get :new, params - offer = user.organizations.first.offers.find(params[:offer]) + it 'finds the transfer offer' do + get :new, params.merge(offer: offer.id) + expect(assigns(:offer)).to eq(offer) + end + end - expect(assigns(:offer)).to eq(offer) + context 'when the offer is not specified' do + it 'does not find any offer' do + get :new, params + expect(assigns(:offer)).to be_nil + end end - end - context 'when the offer is not specified' do - it 'does not find any offer' do - get :new, params - expect(assigns(:offer)).to be_nil + context 'when the user is admin of the current organization' do + let(:user) { member_admin.user } + + it 'finds all accounts in the organization as sources' do + get :new, params + expect(assigns(:sources)).to contain_exactly( + test_organization.account, member_admin.account + ) + end + end + + context 'when the user is not admin of the current organization' do + it 'does not assign :sources' do + get :new, params + expect(assigns(:sources)).to be_nil + end end end - context 'when the user is admin of the current organization' do - let(:user) { member_admin.user } + context 'when the destination is an organization account' do + let(:params) { { id: test_organization.id, organization: true } } - it 'finds all accounts in the organization as sources' do + it 'renders the :new template' do + expect(get :new, params).to render_template(:new_organization) + end + + it 'finds the destination account' do get :new, params - expect(assigns(:sources)).to contain_exactly( - test_organization.account, member_admin.account - ) + expect(assigns(:destination)).to eq(test_organization.account.id) end - end - context 'when the user is not admin of the current organization' do - it 'does not assign :sources' do + context 'when the user is the admin of the current organization' do + let(:user) { member_admin.user } + + it 'renders the page successfully' do + expect(get :new, params).to be_ok + end + end + + it 'finds the transfer source' do get :new, params - expect(assigns(:sources)).to be_nil + source = user.members.find_by(organization: test_organization).account.id + + expect(assigns(:source)).to eq(source) + end + + context 'when an offer is specified' do + let(:offer) { Fabricate(:offer, organization: test_organization) } + + it 'finds the transfer offer' do + get :new, params.merge(offer: offer.id) + expect(assigns(:offer)).to eq(offer) + end end end end From 808ed91479b1a79383f7fb7a0abdbdff93e4f71d Mon Sep 17 00:00:00 2001 From: Pau Perez Date: Wed, 16 Aug 2017 19:43:11 +0200 Subject: [PATCH 04/58] DRY #new by abstracting ivars into their methods This is slowly turning "turning difference into sameness" following Sandi Metz style. --- app/controllers/transfers_controller.rb | 88 +++++++++++-------- spec/controllers/transfers_controller_spec.rb | 8 +- 2 files changed, 57 insertions(+), 39 deletions(-) diff --git a/app/controllers/transfers_controller.rb b/app/controllers/transfers_controller.rb index 64b70eaf5..21c0bdb29 100644 --- a/app/controllers/transfers_controller.rb +++ b/app/controllers/transfers_controller.rb @@ -15,32 +15,13 @@ def create end def new - if for_organization? - load_resource - - @source = find_transfer_source - @offer = find_transfer_offer - @destination = @organization.account.id - @transfer = Transfer.new(source: @source, destination: @destination) - @sources = find_transfer_sources_for_admin + load_resource + @source = find_transfer_source + @offer = find_transfer_offer + @sources = find_transfer_sources_for_admin + @transfer = build_transfer - render :new_organization - else - @user = scoped_users.find(params[:id]) - @destination = @user - .members - .find_by(organization: current_organization) - .account - .id - @source = find_transfer_source - @offer = find_transfer_offer - @transfer = Transfer.new( - source: @source, - destination: @destination, - post: @offer - ) - @sources = find_transfer_sources_for_admin - end + render(template) end def delete_reason @@ -54,20 +35,57 @@ def delete_reason private + def template + if for_organization? + :new_organization + else + :new + end + end + + def build_transfer + if for_organization? + Transfer.new(source: @source, destination: destination) + else + Transfer.new( + source: @source, + destination: destination, + post: @offer + ) + end + end + + def destination + @destination ||= if for_organization? + @organization.account.id + else + @user + .members + .find_by(organization: current_organization) + .account + .id + end + end + def for_organization? params.key?('organization') end def load_resource - if params[:id] - @organization = Organization.find(params[:id]) + if for_organization? + if params[:id] + @organization = Organization.find(params[:id]) + else + @organizations = Organization.all + end else - @organizations = Organization.all + @user = scoped_users.find(params[:id]) end end def find_transfer_sources_for_admin return unless admin? + [current_organization.account] + current_organization.member_accounts.where("members.active is true") end @@ -78,13 +96,13 @@ def find_transfer_offer end def find_transfer_source - if for_organization? - current_user.members. - find_by(organization: @organization).account.id - else - current_user.members. - find_by(organization: current_organization).account.id - end + organization = if for_organization? + @organization + else + current_organization + end + + current_user.members.find_by(organization: organization).account.id end def scoped_users diff --git a/spec/controllers/transfers_controller_spec.rb b/spec/controllers/transfers_controller_spec.rb index ca3e93cc1..3627f59bc 100644 --- a/spec/controllers/transfers_controller_spec.rb +++ b/spec/controllers/transfers_controller_spec.rb @@ -29,9 +29,9 @@ it 'finds the destination account' do get :new, params - account = user.members.find_by(organization: user.organizations.first).account + user_account = user.members.find_by(organization: user.organizations.first).account - expect(assigns(:destination)).to eq(account.id) + expect(assigns(:destination)).to eq(user_account.id) end it 'finds the transfer source' do @@ -98,9 +98,9 @@ it 'finds the transfer source' do get :new, params - source = user.members.find_by(organization: test_organization).account.id + giver_account = user.members.find_by(organization: test_organization).account.id - expect(assigns(:source)).to eq(source) + expect(assigns(:source)).to eq(giver_account) end context 'when an offer is specified' do From 69982abe8a849f7647b649257b4b72c85de3ace8 Mon Sep 17 00:00:00 2001 From: Pau Perez Date: Wed, 16 Aug 2017 20:11:26 +0200 Subject: [PATCH 05/58] Autofix cops and change style of spec --- .rubocop.yml | 2 +- spec/controllers/transfers_controller_spec.rb | 86 ++++++++++--------- 2 files changed, 45 insertions(+), 43 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 9a9c0c4ea..48a00e17b 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -188,7 +188,7 @@ SpecialGlobalVars: Enabled: false StringLiterals: - EnforcedStyle: double_quotes + EnforcedStyle: single_quotes VariableInterpolation: Enabled: false diff --git a/spec/controllers/transfers_controller_spec.rb b/spec/controllers/transfers_controller_spec.rb index 3627f59bc..ae93fcf05 100644 --- a/spec/controllers/transfers_controller_spec.rb +++ b/spec/controllers/transfers_controller_spec.rb @@ -1,12 +1,12 @@ require 'spec_helper' describe TransfersController do - let (:test_organization) { Fabricate(:organization)} - let (:member_admin) { Fabricate(:member, organization: test_organization, manager: true)} + let (:test_organization) { Fabricate(:organization) } + 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) } - include_context "stub browser locale" + include_context 'stub browser locale' before { set_browser_locale('ca') } @@ -114,74 +114,76 @@ end end - describe "POST #create" do + describe 'POST #create' do before { login(user) } - context "with valid params" do - context "with an admin user logged" do + context 'with valid params' do + context 'with an admin user logged' do + subject(:post_create) do + post 'create', transfer: { + source: member_giver.account.id, + destination: member_taker.account.id, + amount: 5 + } + end let(:user) { member_admin.user } - subject { post 'create', transfer: {source: member_giver.account.id, destination: member_taker.account.id, amount: 5} } - it "creates a new Transfer" do - expect { - subject - }.to change(Transfer, :count).by 1 + it 'creates a new Transfer' do + expect { post_create }.to change(Transfer, :count).by 1 end - it "creates two Movements" do - expect { - subject - }.to change { Movement.count}.by 2 + it 'creates two Movements' do + expect { post_create }.to change { Movement.count }.by 2 end - it "updates the balance of both accounts" do - expect { - subject + it 'updates the balance of both accounts' do + expect do + post_create member_giver.reload - }.to change { member_giver.account.balance.to_i }.by -5 + end.to change { member_giver.account.balance.to_i }.by -5 - expect { - subject + expect do + post_create member_taker.reload - }.to change { member_taker.account.balance.to_i }.by 5 + end.to change { member_taker.account.balance.to_i }.by 5 end end - context "with a regular user logged" do + context 'with a regular user logged' do + subject(:post_create) do + post 'create', transfer: { + destination: member_taker.account.id, + amount: 5 + } + end let(:user) { member_giver.user } - subject { post 'create', transfer: {destination: member_taker.account.id, amount: 5} } - it "creates a new Transfer" do - expect { - subject - }.to change(Transfer, :count).by 1 + it 'creates a new Transfer' do + expect { post_create }.to change(Transfer, :count).by 1 end - it "creates two Movements" do - expect { - subject - }.to change { Movement.count}.by 2 + it 'creates two Movements' do + expect { post_create }.to change { Movement.count }.by 2 end - it "updates the balance of both accounts" do - expect { - subject + it 'updates the balance of both accounts' do + expect do + post_create member_giver.reload - }.to change { member_giver.account.balance.to_i }.by -5 + end.to change { member_giver.account.balance.to_i }.by -5 - expect { - subject + expect do + post_create member_taker.reload - }.to change { member_taker.account.balance.to_i }.by 5 + end.to change { member_taker.account.balance.to_i }.by 5 end - it "redirects to destination" do - expect(subject).to redirect_to(member_taker.user) + it 'redirects to destination' do + expect(post_create).to redirect_to(member_taker.user) end end end - end end From 0aa40730c64321e8fd8d573d111c74221c86215a Mon Sep 17 00:00:00 2001 From: Pau Perez Date: Wed, 16 Aug 2017 20:20:03 +0200 Subject: [PATCH 06/58] Simplify #create action --- app/controllers/transfers_controller.rb | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/app/controllers/transfers_controller.rb b/app/controllers/transfers_controller.rb index 21c0bdb29..96843b179 100644 --- a/app/controllers/transfers_controller.rb +++ b/app/controllers/transfers_controller.rb @@ -6,12 +6,10 @@ def create transfer_params.merge(source: @source, destination: @account) ) - begin - transfer.save! - rescue ActiveRecord::RecordInvalid - flash[:error] = transfer.errors.full_messages.to_sentence - end + transfer.save! redirect_to redirect_target + rescue ActiveRecord::RecordInvalid + flash[:error] = transfer.errors.full_messages.to_sentence end def new From 0fa4668facfe0d802c2429d980c029bc47760a9b Mon Sep 17 00:00:00 2001 From: Pau Perez Date: Wed, 16 Aug 2017 20:20:47 +0200 Subject: [PATCH 07/58] Remove dead #give_time actions --- app/controllers/organizations_controller.rb | 24 ------------------ app/controllers/users_controller.rb | 28 --------------------- 2 files changed, 52 deletions(-) diff --git a/app/controllers/organizations_controller.rb b/app/controllers/organizations_controller.rb index 0e3ab87a7..1d84b947d 100644 --- a/app/controllers/organizations_controller.rb +++ b/app/controllers/organizations_controller.rb @@ -48,14 +48,6 @@ def destroy redirect_to organizations_path, notice: "deleted" end - def give_time - @destination = @organization.account.id - @source = find_transfer_source - @offer = find_transfer_offer - @transfer = Transfer.new(source: @source, destination: @destination) - @sources = find_transfer_sources_for_admin - end - def set_current if current_user session[:current_organization_id] = @organization.id @@ -70,20 +62,4 @@ def organization_params public_opening_times description address neighborhood city domain]) end - - def find_transfer_offer - current_organization.offers. - find(params[:offer]) if params[:offer].present? - end - - def find_transfer_source - current_user.members. - find_by(organization: @organization).account.id - end - - def find_transfer_sources_for_admin - return unless admin? - [current_organization.account] + - current_organization.member_accounts.where("members.active is true") - end end diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 68f6fb828..89f2cd6ad 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -61,18 +61,6 @@ def update end end - def give_time - @user = scoped_users.find(params[:id]) - @destination = @user.members. - find_by(organization: current_organization).account.id - @source = find_transfer_source - @offer = find_transfer_offer - @transfer = Transfer.new(source: @source, - destination: @destination, - post: @offer) - @sources = find_transfer_sources_for_admin - end - private def user_params @@ -85,22 +73,6 @@ def user_params params.require(:user).permit *fields_to_permit end - def find_transfer_offer - current_organization.offers. - find(params[:offer]) if params[:offer].present? - end - - def find_transfer_source - current_user.members. - find_by(organization: current_organization).account.id - end - - def find_transfer_sources_for_admin - return unless admin? - [current_organization.account] + - current_organization.member_accounts.where("members.active is true") - end - def find_user if current_user.id == params[:id].to_i current_user From fa3adf429fb134fc47315b6801ca1a770c6685ad Mon Sep 17 00:00:00 2001 From: Pau Perez Date: Wed, 16 Aug 2017 21:34:49 +0200 Subject: [PATCH 08/58] Abstract transfers controller by passing account By specifying the account id we can abstract away from User or Organization objects and trust just the account. As a result, this removes ifs and therefore complexity. --- app/controllers/transfers_controller.rb | 66 +++++++++---------- spec/controllers/transfers_controller_spec.rb | 38 ++++++++--- 2 files changed, 60 insertions(+), 44 deletions(-) diff --git a/app/controllers/transfers_controller.rb b/app/controllers/transfers_controller.rb index 96843b179..28dc2a9e1 100644 --- a/app/controllers/transfers_controller.rb +++ b/app/controllers/transfers_controller.rb @@ -41,43 +41,40 @@ def template end end + # Returns a new instance of Transfer with the data provided in the request + # + # @return [Transfer] def build_transfer - if for_organization? - Transfer.new(source: @source, destination: destination) - else - Transfer.new( - source: @source, - destination: destination, - post: @offer - ) - end + transfer = Transfer.new(source: @source, destination: destination_account.id) + transfer.post = @offer unless for_organization? + transfer end - def destination - @destination ||= if for_organization? - @organization.account.id - else - @user - .members - .find_by(organization: current_organization) - .account - .id - end + # TODO: this method implements authorization by scoping the destination + # account in all the accounts of the current organization. If the specified + # destination account does not belong to it, the request will simply faily. + # + # Returns the account the time will be transfered to + # + # @return [Account] + def destination_account + @destination_account ||= current_organization + .all_accounts + .find(params[:destination_account_id]) end + # Checks whether the destination account is an organization + # + # @return [Boolean] def for_organization? - params.key?('organization') + destination_account.accountable.class == Organization end def load_resource if for_organization? - if params[:id] - @organization = Organization.find(params[:id]) - else - @organizations = Organization.all - end + @organization = destination_account.accountable else - @user = scoped_users.find(params[:id]) + @user = destination_account.accountable.user end end @@ -88,25 +85,22 @@ def find_transfer_sources_for_admin current_organization.member_accounts.where("members.active is true") end + # Returns the offer that is the subject of the transfer + # + # @return [Offer] def find_transfer_offer current_organization.offers. find(params[:offer]) if params[:offer].present? end + # Returns the id of the account that acts as source of the transfer + # + # @return [Integer] def find_transfer_source - organization = if for_organization? - @organization - else - current_organization - end - + organization = @organization || current_organization current_user.members.find_by(organization: organization).account.id end - def scoped_users - current_organization.users - end - def find_source if admin? Account.find(transfer_params[:source]) diff --git a/spec/controllers/transfers_controller_spec.rb b/spec/controllers/transfers_controller_spec.rb index ae93fcf05..a06c53f78 100644 --- a/spec/controllers/transfers_controller_spec.rb +++ b/spec/controllers/transfers_controller_spec.rb @@ -16,7 +16,13 @@ before { login(user) } context 'when the destination is a user account' do - let(:params) { { id: user.id } } + let(:user_account) { user.members.find_by(organization: user.organizations.first).account } + let(:params) do + { + id: user.id, + destination_account_id: user_account.id + } + end it 'renders the :new template' do expect(get :new, params).to render_template(:new) @@ -29,16 +35,17 @@ it 'finds the destination account' do get :new, params - user_account = user.members.find_by(organization: user.organizations.first).account - - expect(assigns(:destination)).to eq(user_account.id) + expect(assigns(:destination_account)).to eq(user_account) end it 'finds the transfer source' do get :new, params - source = user.members.find_by(organization: user.organizations.first).account.id + expect(assigns(:source)).to eq(user_account.id) + end - expect(assigns(:source)).to eq(source) + it 'builds a transfer with the id of the destination' do + get :new, params + expect(assigns(:transfer).destination).to eq(user_account.id) end context 'when the offer is specified' do @@ -48,6 +55,11 @@ get :new, params.merge(offer: offer.id) expect(assigns(:offer)).to eq(offer) end + + it 'builds a transfer with the offer as post' do + get :new, params.merge(offer: offer.id) + expect(assigns(:transfer).post).to eq(offer) + end end context 'when the offer is not specified' do @@ -77,7 +89,12 @@ end context 'when the destination is an organization account' do - let(:params) { { id: test_organization.id, organization: true } } + let(:params) do + { + id: test_organization.id, + destination_account_id: test_organization.account.id + } + end it 'renders the :new template' do expect(get :new, params).to render_template(:new_organization) @@ -85,7 +102,12 @@ it 'finds the destination account' do get :new, params - expect(assigns(:destination)).to eq(test_organization.account.id) + expect(assigns(:destination_account)).to eq(test_organization.account) + end + + it 'builds a transfer with the id of the destination' do + get :new, params + expect(assigns(:transfer).destination).to eq(test_organization.account.id) end context 'when the user is the admin of the current organization' do From 24e5d6ad9897c822943883082e9c5f097a0ae314 Mon Sep 17 00:00:00 2001 From: Pau Perez Date: Wed, 16 Aug 2017 21:54:43 +0200 Subject: [PATCH 09/58] Use @accountable instead of @user or @organization Then each particular view knows what type the accountable has and uses the appropriate methods like user_path or organization_path. --- app/controllers/transfers_controller.rb | 18 +++++++++--------- app/views/transfers/new.html.erb | 4 ++-- app/views/transfers/new_organization.html.erb | 2 +- spec/controllers/transfers_controller_spec.rb | 9 +++++++-- 4 files changed, 19 insertions(+), 14 deletions(-) diff --git a/app/controllers/transfers_controller.rb b/app/controllers/transfers_controller.rb index 28dc2a9e1..1114d7e06 100644 --- a/app/controllers/transfers_controller.rb +++ b/app/controllers/transfers_controller.rb @@ -13,7 +13,6 @@ def create end def new - load_resource @source = find_transfer_source @offer = find_transfer_offer @sources = find_transfer_sources_for_admin @@ -70,12 +69,8 @@ def for_organization? destination_account.accountable.class == Organization end - def load_resource - if for_organization? - @organization = destination_account.accountable - else - @user = destination_account.accountable.user - end + def accountable + @accountable ||= destination_account.accountable end def find_transfer_sources_for_admin @@ -93,11 +88,16 @@ def find_transfer_offer find(params[:offer]) if params[:offer].present? end - # Returns the id of the account that acts as source of the transfer + # Returns the id of the account that acts as source of the transfer. + # Either the account of the organization or the account of the current user. # # @return [Integer] def find_transfer_source - organization = @organization || current_organization + organization = if accountable.is_a?(Organization) + accountable + else + current_organization + end current_user.members.find_by(organization: organization).account.id end diff --git a/app/views/transfers/new.html.erb b/app/views/transfers/new.html.erb index 2b5b143df..e0c6b8f74 100644 --- a/app/views/transfers/new.html.erb +++ b/app/views/transfers/new.html.erb @@ -2,7 +2,7 @@ <%= t ".give_time" %> - <%= link_to @user.member(current_organization).display_name_with_uid, user_path(@user) %> + <%= link_to @accountable.display_name_with_uid, user_path(@accountable.user) %> <% if @offer %>

@@ -49,7 +49,7 @@ <%= f.input :post, readonly: true %> <%= f.input :post_id, as: :hidden %> <% else %> - <%= f.input :post_id, collection: @user.member(current_organization).offers.active %> + <%= f.input :post_id, collection: @accountable.offers.active %> <% end %>
diff --git a/app/views/transfers/new_organization.html.erb b/app/views/transfers/new_organization.html.erb index 02787bdef..94d8a3e78 100644 --- a/app/views/transfers/new_organization.html.erb +++ b/app/views/transfers/new_organization.html.erb @@ -1,6 +1,6 @@

<%= t '.give_time' %> - <%= link_to @organization, organization_path(@organization) %> + <%= link_to @accountable, organization_path(@accountable) %>

<% if @offer %>

diff --git a/spec/controllers/transfers_controller_spec.rb b/spec/controllers/transfers_controller_spec.rb index a06c53f78..ae4f7fead 100644 --- a/spec/controllers/transfers_controller_spec.rb +++ b/spec/controllers/transfers_controller_spec.rb @@ -28,9 +28,9 @@ expect(get :new, params).to render_template(:new) end - it 'finds the user' do + it 'finds the accountable' do get :new, params - expect(assigns(:user)).to eq(user) + expect(assigns(:accountable)).to eq(member_giver) end it 'finds the destination account' do @@ -100,6 +100,11 @@ expect(get :new, params).to render_template(:new_organization) end + it 'finds the accountable' do + get :new, params + expect(assigns(:accountable)).to eq(test_organization) + end + it 'finds the destination account' do get :new, params expect(assigns(:destination_account)).to eq(test_organization.account) From 96f36b14617f105b5b0d54cfdc8dbb3de0b9ab00 Mon Sep 17 00:00:00 2001 From: Pau Perez Date: Thu, 17 Aug 2017 13:11:31 +0200 Subject: [PATCH 10/58] Refactor assigns' specs to check HTML instead This makes the controller specs integration-like enough to ensure things keep working when we later refactor the views and the controller ivars. --- app/views/transfers/new.html.erb | 4 +- app/views/transfers/new_organization.html.erb | 4 +- spec/controllers/transfers_controller_spec.rb | 49 ++++++++++--------- 3 files changed, 27 insertions(+), 30 deletions(-) diff --git a/app/views/transfers/new.html.erb b/app/views/transfers/new.html.erb index e0c6b8f74..3b33ac4f8 100644 --- a/app/views/transfers/new.html.erb +++ b/app/views/transfers/new.html.erb @@ -5,9 +5,7 @@ <%= link_to @accountable.display_name_with_uid, user_path(@accountable.user) %>

<% if @offer %> -

- <%= @offer %> -

+

<%= @offer %>

<% end %> <%= simple_form_for @transfer do |f| %>
diff --git a/app/views/transfers/new_organization.html.erb b/app/views/transfers/new_organization.html.erb index 94d8a3e78..5de3c77fa 100644 --- a/app/views/transfers/new_organization.html.erb +++ b/app/views/transfers/new_organization.html.erb @@ -3,9 +3,7 @@ <%= link_to @accountable, organization_path(@accountable) %>

<% if @offer %> -

- <%= @offer %> -

+

<%= @offer %>

<% end %> <%= simple_form_for @transfer do |f| %>
diff --git a/spec/controllers/transfers_controller_spec.rb b/spec/controllers/transfers_controller_spec.rb index ae4f7fead..60793ade4 100644 --- a/spec/controllers/transfers_controller_spec.rb +++ b/spec/controllers/transfers_controller_spec.rb @@ -30,22 +30,19 @@ it 'finds the accountable' do get :new, params - expect(assigns(:accountable)).to eq(member_giver) + expect(response.body) + .to include("#{member_giver.display_name_with_uid}") end it 'finds the destination account' do get :new, params - expect(assigns(:destination_account)).to eq(user_account) - end - - it 'finds the transfer source' do - get :new, params - expect(assigns(:source)).to eq(user_account.id) + expect(response.body).to include("") end it 'builds a transfer with the id of the destination' do get :new, params - expect(assigns(:transfer).destination).to eq(user_account.id) + expect(response.body) + .to include("") end context 'when the offer is specified' do @@ -53,19 +50,19 @@ it 'finds the transfer offer' do get :new, params.merge(offer: offer.id) - expect(assigns(:offer)).to eq(offer) + expect(response.body).to include("

#{offer}

") end it 'builds a transfer with the offer as post' do get :new, params.merge(offer: offer.id) - expect(assigns(:transfer).post).to eq(offer) + expect(response.body).to include("") end end context 'when the offer is not specified' do it 'does not find any offer' do get :new, params - expect(assigns(:offer)).to be_nil + expect(response.body).to include('
diff --git a/app/views/transfers/new_organization.html.erb b/app/views/transfers/new_organization.html.erb index 5de3c77fa..4953d5023 100644 --- a/app/views/transfers/new_organization.html.erb +++ b/app/views/transfers/new_organization.html.erb @@ -1,11 +1,11 @@

<%= t '.give_time' %> - <%= link_to @accountable, organization_path(@accountable) %> + <%= link_to accountable, organization_path(accountable) %>

-<% if @offer %> -

<%= @offer %>

+<% if offer %> +

<%= offer %>

<% end %> -<%= simple_form_for @transfer do |f| %> +<%= simple_form_for transfer do |f| %>
<%= f.input :hours, as: :integer, @@ -33,7 +33,7 @@ <%# TODO: refactor. it's very complex for a view. %> <%= f.select :source, options_for_select( - @sources.sort_by { |source| [source.accountable_type, source.accountable_id] } + sources.sort_by { |source| [source.accountable_type, source.accountable_id] } .map { |a| ["#{a.accountable_type == 'Member' ? a.accountable.member_uid : a.accountable_id} #{a.accountable_type} #{a.accountable.to_s}", a.id] }, selected: current_user.member(current_organization).account.id ), @@ -42,7 +42,7 @@ class: 'form-control' %>
<% end %> - <% if @offer %> + <% if offer %> <%= f.input :post, as: :hidden %> <% end %>
From bf6223b270aef24ead2fde1b4146e53cc7743334 Mon Sep 17 00:00:00 2001 From: Pau Perez Date: Sat, 19 Aug 2017 13:31:21 +0200 Subject: [PATCH 12/58] Extract transfer sources select --- app/models/transfer_sources_options.rb | 24 +++++++++++++++++++ app/views/transfers/_sources.html.erb | 9 +++++++ app/views/transfers/new.html.erb | 16 ++++--------- app/views/transfers/new_organization.html.erb | 21 ++++------------ 4 files changed, 42 insertions(+), 28 deletions(-) create mode 100644 app/models/transfer_sources_options.rb create mode 100644 app/views/transfers/_sources.html.erb diff --git a/app/models/transfer_sources_options.rb b/app/models/transfer_sources_options.rb new file mode 100644 index 000000000..5f730f5a3 --- /dev/null +++ b/app/models/transfer_sources_options.rb @@ -0,0 +1,24 @@ +class TransferSourcesOptions + def initialize(sources, accountable) + @sources = sources + @accountable = accountable + end + + def to_a + sources + .sort_by { |source| [source.accountable_type, source.accountable.try(:member_uid)] } + .map { |a| ["#{a.accountable_type == "Member" ? a.accountable.member_uid : accountable_option_name(a)} #{a.accountable_type} #{a.accountable.to_s}", a.id] } + end + + private + + attr_reader :sources, :accountable + + def accountable_option_name(account) + if accountable.is_a?(Organization) + account.accountable_id + else + '' + end + end +end diff --git a/app/views/transfers/_sources.html.erb b/app/views/transfers/_sources.html.erb new file mode 100644 index 000000000..271092838 --- /dev/null +++ b/app/views/transfers/_sources.html.erb @@ -0,0 +1,9 @@ +
+ <%= form.label :source %> + <%= form.select :source, + options_for_select( + TransferSourcesOptions.new(sources, accountable).to_a, + selected: current_user.member(current_organization).account.id + ), {}, id: "select2-time", class: "form-control" + %> +
diff --git a/app/views/transfers/new.html.erb b/app/views/transfers/new.html.erb index a36af9eb4..a9c2916fb 100644 --- a/app/views/transfers/new.html.erb +++ b/app/views/transfers/new.html.erb @@ -29,26 +29,18 @@ <%= f.input :amount, as: :hidden %> <%= f.input :reason %> <%= f.input :destination, as: :hidden %> + <% if admin? %> -
- <%= f.label :source %> - <%= f.select :source, - options_for_select( - sources.sort_by { |source| [source.accountable_type, source.accountable.try(:member_uid)] } - .map { |a| ["#{a.accountable_type == "Member" ? a.accountable.member_uid : ""} #{a.accountable_type} #{a.accountable.to_s}", a.id] }, - selected: current_user.member(current_organization).account.id - ), - {}, - id: "select2-time", - class: "form-control" %> -
+ <%= render partial: 'sources', locals: { form: f, sources: sources, accountable: accountable} %> <% end %> + <% if offer %> <%= f.input :post, readonly: true %> <%= f.input :post_id, as: :hidden %> <% else %> <%= f.input :post_id, collection: accountable.offers.active %> <% end %> +
<%= f.button :submit, class: "btn btn-default", style: "margin-bottom: 20px;" %> diff --git a/app/views/transfers/new_organization.html.erb b/app/views/transfers/new_organization.html.erb index 4953d5023..e31e8fa16 100644 --- a/app/views/transfers/new_organization.html.erb +++ b/app/views/transfers/new_organization.html.erb @@ -27,29 +27,18 @@ <%= f.input :amount, as: :hidden %> <%= f.input :reason %> <%= f.input :destination, as: :hidden %> + <% if admin? %> -
- <%= f.label :source %> - <%# TODO: refactor. it's very complex for a view. %> - <%= f.select :source, - options_for_select( - sources.sort_by { |source| [source.accountable_type, source.accountable_id] } - .map { |a| ["#{a.accountable_type == 'Member' ? a.accountable.member_uid : a.accountable_id} #{a.accountable_type} #{a.accountable.to_s}", a.id] }, - selected: current_user.member(current_organization).account.id - ), - {}, - id: 'select2-time', - class: 'form-control' %> -
+ <%= render partial: 'sources', locals: { form: f, sources: sources, accountable: accountable } %> <% end %> + <% if offer %> <%= f.input :post, as: :hidden %> <% end %>
+
- <%= f.button :submit, - class: 'btn btn-default', - style: 'margin-bottom: 20px;' %> + <%= f.button :submit, class: 'btn btn-default', style: 'margin-bottom: 20px;' %>
<% end %> diff --git a/spec/controllers/transfers_controller_spec.rb b/spec/controllers/transfers_controller_spec.rb index 60793ade4..6bd3bf026 100644 --- a/spec/controllers/transfers_controller_spec.rb +++ b/spec/controllers/transfers_controller_spec.rb @@ -24,10 +24,6 @@ } end - it 'renders the :new template' do - expect(get :new, params).to render_template(:new) - end - it 'finds the accountable' do get :new, params expect(response.body) @@ -94,10 +90,6 @@ } end - it 'renders the :new template' do - expect(get :new, params).to render_template(:new_organization) - end - it 'finds the accountable' do get :new, params expect(response.body) From 14eaad80d1ff3811071637e804d9c57bbcb0206e Mon Sep 17 00:00:00 2001 From: Pau Perez Date: Thu, 24 Aug 2017 19:22:07 +0200 Subject: [PATCH 15/58] Extract option building into class + polymorphism --- app/models/account_option_tag.rb | 19 +++++++++++++++++ app/models/member.rb | 4 ++++ app/models/organization.rb | 8 ++++++++ app/models/transfer_sources_options.rb | 28 ++++++++++++++------------ 4 files changed, 46 insertions(+), 13 deletions(-) create mode 100644 app/models/account_option_tag.rb diff --git a/app/models/account_option_tag.rb b/app/models/account_option_tag.rb new file mode 100644 index 000000000..6956773dd --- /dev/null +++ b/app/models/account_option_tag.rb @@ -0,0 +1,19 @@ +class AccountOptionTag + def initialize(accountable, destination_accountable) + @accountable = accountable + @destination_accountable = destination_accountable + end + + def to_s + "#{display_id} #{accountable.class} #{accountable}" + end + + private + + attr_reader :accountable, :destination_accountable + + def display_id + accountable.display_id(destination_accountable) + end +end + diff --git a/app/models/member.rb b/app/models/member.rb index a9f996fa0..bb5c6e448 100644 --- a/app/models/member.rb +++ b/app/models/member.rb @@ -57,6 +57,10 @@ def inquiries Post.where(organization: organization, user: user, type: "Inquiry") end + def display_id(_destination_accountable) + member_uid + end + private def remove_orphaned_users diff --git a/app/models/organization.rb b/app/models/organization.rb index d3222e7c8..a5147aeca 100644 --- a/app/models/organization.rb +++ b/app/models/organization.rb @@ -57,4 +57,12 @@ def ensure_url errors.add(:web, :url_format_invalid) end end + + def display_id(destination_accountable) + if destination_accountable.is_a?(Organization) + account.accountable_id + else + '' + end + end end diff --git a/app/models/transfer_sources_options.rb b/app/models/transfer_sources_options.rb index 5f730f5a3..2db2e66dc 100644 --- a/app/models/transfer_sources_options.rb +++ b/app/models/transfer_sources_options.rb @@ -1,24 +1,26 @@ class TransferSourcesOptions - def initialize(sources, accountable) + def initialize(sources, destination_accountable) @sources = sources - @accountable = accountable + @destination_accountable = destination_accountable end def to_a sources - .sort_by { |source| [source.accountable_type, source.accountable.try(:member_uid)] } - .map { |a| ["#{a.accountable_type == "Member" ? a.accountable.member_uid : accountable_option_name(a)} #{a.accountable_type} #{a.accountable.to_s}", a.id] } + .sort_by do |source| + [ + source.accountable_type, + source.accountable.try(:member_uid) + ] + end + .map do |account| + [ + AccountOptionTag.new(account.accountable, destination_accountable).to_s, + account.id + ] + end end private - attr_reader :sources, :accountable - - def accountable_option_name(account) - if accountable.is_a?(Organization) - account.accountable_id - else - '' - end - end + attr_reader :sources, :destination_accountable end From be04d97cc7eacd074b79f7edf85c984bfaf0ce3b Mon Sep 17 00:00:00 2001 From: Pau Perez Date: Thu, 24 Aug 2017 19:55:53 +0200 Subject: [PATCH 16/58] Extract TransferFactory and decouple view --- app/controllers/transfers_controller.rb | 78 ++++-------------------- app/models/transfer_factory.rb | 79 +++++++++++++++++++++++++ app/views/transfers/new.html.erb | 2 +- 3 files changed, 92 insertions(+), 67 deletions(-) create mode 100644 app/models/transfer_factory.rb diff --git a/app/controllers/transfers_controller.rb b/app/controllers/transfers_controller.rb index e72f67747..8213a3f87 100644 --- a/app/controllers/transfers_controller.rb +++ b/app/controllers/transfers_controller.rb @@ -13,10 +13,17 @@ def create end def new - source = find_transfer_source - offer = find_transfer_offer - sources = find_transfer_sources_for_admin - transfer = build_transfer(offer, source) + transfer_factory = TransferFactory.new( + current_organization, + current_user, + params[:offer], + params[:destination_account_id] + ) + + offer = transfer_factory.offer + transfer = transfer_factory.build_transfer + transfer_sources = transfer_factory.transfer_sources + accountable = transfer_factory.accountable render( :new, @@ -24,7 +31,7 @@ def new accountable: accountable, transfer: transfer, offer: offer, - sources: sources + sources: transfer_sources } ) end @@ -40,67 +47,6 @@ def delete_reason private - # Returns a new instance of Transfer with the data provided in the request - # - # @return [Transfer] - def build_transfer(offer, source) - transfer = Transfer.new(source: source, destination: destination_account.id) - transfer.post = offer unless for_organization? - transfer - end - - # TODO: this method implements authorization by scoping the destination - # account in all the accounts of the current organization. If the specified - # destination account does not belong to it, the request will simply faily. - # - # Returns the account the time will be transfered to - # - # @return [Account] - def destination_account - @destination_account ||= current_organization - .all_accounts - .find(params[:destination_account_id]) - end - - # Checks whether the destination account is an organization - # - # @return [Boolean] - def for_organization? - destination_account.accountable.class == Organization - end - - def accountable - @accountable ||= destination_account.accountable - end - - def find_transfer_sources_for_admin - return unless admin? - - [current_organization.account] + - current_organization.member_accounts.where("members.active is true") - end - - # Returns the offer that is the subject of the transfer - # - # @return [Offer] - def find_transfer_offer - current_organization.offers. - find(params[:offer]) if params[:offer].present? - end - - # Returns the id of the account that acts as source of the transfer. - # Either the account of the organization or the account of the current user. - # - # @return [Integer] - def find_transfer_source - organization = if accountable.is_a?(Organization) - accountable - else - current_organization - end - current_user.members.find_by(organization: organization).account.id - end - def find_source if admin? Account.find(transfer_params[:source]) diff --git a/app/models/transfer_factory.rb b/app/models/transfer_factory.rb new file mode 100644 index 000000000..088b04511 --- /dev/null +++ b/app/models/transfer_factory.rb @@ -0,0 +1,79 @@ +class TransferFactory + def initialize(current_organization, current_user, offer_id, destination_account_id) + @current_organization = current_organization + @current_user = current_user + @offer_id = offer_id + @destination_account_id = destination_account_id + end + + # Returns the offer that is the subject of the transfer + # + # @return [Offer] + def offer + current_organization.offers. + find(offer_id) if offer_id.present? + end + + # Returns a new instance of Transfer with the data provided in the request + # + # @return [Transfer] + def build_transfer + transfer = Transfer.new(source: source, destination: destination_account.id) + transfer.post = offer unless for_organization? + transfer + end + + def transfer_sources + if admin? + [current_organization.account] + + current_organization.member_accounts.merge(Member.active) + else + [] + end + end + + def accountable + @accountable ||= destination_account.accountable + end + + private + + attr_reader :current_organization, :current_user, :offer_id, :destination_account_id + + # Returns the id of the account that acts as source of the transfer. + # Either the account of the organization or the account of the current user. + # + # @return [Integer] + def source + organization = if accountable.is_a?(Organization) + accountable + else + current_organization + end + current_user.members.find_by(organization: organization).account.id + end + + # Checks whether the destination account is an organization + # + # @return [Boolean] + def for_organization? + destination_account.accountable.class == Organization + end + + def admin? + current_user.try :manages?, current_organization + end + + # TODO: this method implements authorization by scoping the destination + # account in all the accounts of the current organization. If the specified + # destination account does not belong to it, the request will simply faily. + # + # Returns the account the time will be transfered to + # + # @return [Account] + def destination_account + @destination_account ||= current_organization + .all_accounts + .find(destination_account_id) + end +end diff --git a/app/views/transfers/new.html.erb b/app/views/transfers/new.html.erb index 3702054b6..66c74131a 100644 --- a/app/views/transfers/new.html.erb +++ b/app/views/transfers/new.html.erb @@ -28,7 +28,7 @@ <%= f.input :reason %> <%= f.input :destination, as: :hidden %> - <% if admin? %> + <% if sources.present? %> <%= render partial: 'sources', locals: { form: f, sources: sources, accountable: accountable } %> <% end %> From e97d4675f2a573ec28d373d6132b0465b73b3a6a Mon Sep 17 00:00:00 2001 From: Pau Perez Date: Sat, 2 Sep 2017 19:57:55 +0200 Subject: [PATCH 17/58] Replace conditional with #find_by_id --- app/models/transfer_factory.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app/models/transfer_factory.rb b/app/models/transfer_factory.rb index 088b04511..7c3439554 100644 --- a/app/models/transfer_factory.rb +++ b/app/models/transfer_factory.rb @@ -8,10 +8,9 @@ def initialize(current_organization, current_user, offer_id, destination_account # Returns the offer that is the subject of the transfer # - # @return [Offer] + # @return [Maybe] def offer - current_organization.offers. - find(offer_id) if offer_id.present? + current_organization.offers.find_by_id(offer_id) end # Returns a new instance of Transfer with the data provided in the request @@ -38,7 +37,8 @@ def accountable private - attr_reader :current_organization, :current_user, :offer_id, :destination_account_id + attr_reader :current_organization, :current_user, :offer_id, + :destination_account_id # Returns the id of the account that acts as source of the transfer. # Either the account of the organization or the account of the current user. From 0130570d5578311a9700a7d40b456111093c7f1c Mon Sep 17 00:00:00 2001 From: Victor Martinez Date: Wed, 18 Oct 2017 17:33:08 +0200 Subject: [PATCH 18/58] Implement pagination & search for users list --- app/assets/javascripts/app/app.js.coffee | 49 --------- app/controllers/users_controller.rb | 12 ++- app/helpers/users_helper.rb | 36 ++----- app/views/users/_user_rows.html.erb | 123 +++++++++++------------ app/views/users/index.html.erb | 73 ++------------ app/views/users/index.js.erb | 2 - config/locales/ca.yml | 6 +- config/locales/en.yml | 56 +++++------ config/locales/es.yml | 14 +-- config/locales/pt-BR.yml | 6 +- 10 files changed, 127 insertions(+), 250 deletions(-) delete mode 100644 app/views/users/index.js.erb diff --git a/app/assets/javascripts/app/app.js.coffee b/app/assets/javascripts/app/app.js.coffee index bee55ffcb..3f41c7d5b 100644 --- a/app/assets/javascripts/app/app.js.coffee +++ b/app/assets/javascripts/app/app.js.coffee @@ -1,51 +1,2 @@ -angular.module('timeoverflow').controller 'UserListCtrl', ["$scope", "$modal", "$http", "$location", ($scope, $modal, $http, $location) -> - - $scope.sortBy = (field) -> - $scope.sort = if $scope.sort == field then "-#{field}" else field - $scope.filterTerm = '' - $scope.$location = $location - - ['filterTerm', 'sort'].map (prop) -> - Object.defineProperty($scope, prop, - get: -> - if prop == 'sort' and $location.search()[prop] == undefined - 'member_id' - else - $location.search()[prop] - set: (val) -> $location.search(prop, val || null) - ) - - $scope.toggle_manager = (user) -> - $modal.open( - templateUrl: 'confirm_toggle_manager.html' - size: 'sm' - scope: $scope - controller: ["$scope", ($scope) -> $scope.username = user.username] - ).result - .then(-> $http.put(user.toggle_manager_link)) - .then(-> user.manager = !user.manager) - - $scope.toggle_active = (user) -> - $modal.open( - templateUrl: 'confirm_toggle_active.html' - size: 'sm' - scope: $scope - controller: ["$scope", ($scope) -> $scope.username = user.username] - ).result - .then(-> $http.put(user.toggle_active_link)) - .then(-> user.active = !user.active) -] - # override this in a view where the organizations are needed angular.module('timeoverflow').value 'Organizations', [] - -angular.module('timeoverflow').filter 'timeBalance', -> - (seconds) -> - if seconds isnt 0 - minutes = Math.abs(seconds) / 60 - hours = (minutes / 60) >> 0 - minutes %= 60 - minutes >>= 0; - if seconds < 0 then "-#{hours}:#{minutes}" else "#{hours}:#{minutes}" - else - "—" diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 68f6fb828..6d818c160 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -1,12 +1,10 @@ class UsersController < ApplicationController before_filter :authenticate_user! - def scoped_users - current_organization.users - end - def index - @users = scoped_users + @search = scoped_users.ransack(params[:q]) + @users = @search.result(distinct: false).page(params[:page]).per(25) + @memberships = current_organization.members. where(user_id: @users.map(&:id)). includes(:account).each_with_object({}) do |mem, ob| @@ -75,6 +73,10 @@ def give_time private + def scoped_users + current_organization.users + end + def user_params fields_to_permit = %w"gender username email date_of_birth phone alt_phone active description notifications" diff --git a/app/helpers/users_helper.rb b/app/helpers/users_helper.rb index 947b95fa0..ac3d541a0 100644 --- a/app/helpers/users_helper.rb +++ b/app/helpers/users_helper.rb @@ -1,34 +1,14 @@ module UsersHelper - # TODO refactor or eliminate - poosibly the second. - def users_as_json - @users = (admin? || superadmin?) ? @users : @users.actives - @users.map do |user| - membership = @memberships[user.id] - { - id: user.id, - avatar: avatar_url(user), - member_id: membership.member_uid, - username: user.username, - email: user.email_if_real, - unconfirmed_email: user.unconfirmed_email, - phone: user.phone, - alt_phone: user.alt_phone, - balance: membership.account_balance.to_i, - - url: user_path(user), - edit_link: edit_user_path(user), - cancel_link: cancel_member_path(membership), - toggle_manager_link: toggle_manager_member_path(membership), - manager: !!membership.manager, - toggle_active_link: toggle_active_member_path(membership), - active: membership.active?, - valid_email: user.has_valid_email? - } - end.to_json.html_safe - end - private + def time_balance(seconds) + if seconds.zero? + "—" + else + [seconds / (60 * 60), (seconds / 60) % 60].map{|value| value.to_s.rjust(2, "0") }.join(":") + end + end + def edit_user_path(user) can_edit_user?(user) ? super : "" end diff --git a/app/views/users/_user_rows.html.erb b/app/views/users/_user_rows.html.erb index 0f7ada7b8..d1f305e46 100644 --- a/app/views/users/_user_rows.html.erb +++ b/app/views/users/_user_rows.html.erb @@ -1,64 +1,63 @@ - - - - - {{user.member_id}} - - - {{user.username}} - - - - {{user.email}} - - - {{user.unconfirmed_email}} - - - {{user.phone}} - {{user.alt_phone}} - {{user.balance | timeBalance}} - <% if current_user.manages?(current_organization) %> - - - <%= glyph :pencil %> - <%= t "global.edit" %> - - "> - <%= glyph :ban_circle %> - <%= t "global.cancel_membership" %> - - - - <%= glyph :arrow_down %> - <%= t "global.demote" %> - - - <%= glyph :arrow_up %> - <%= t "global.promote" %> - - - - - <%= glyph :remove %> - <%= t ".deactivate" %> - - - <%= glyph :ok %> - <%= t ".activate" %> - - +<% users.each do |user| %> + <% membership = @memberships[user.id] %> + + <%= content_tag(:tr, class: membership.active? ? "" : "bg-danger") do %> + + <%= image_tag avatar_url(user, 32), width: 32, height: 32 %> + <%= membership.member_uid %> + + <% if !membership.active? %><% end %> + <%= link_to user.username, user_path(user) %> + + + <% if user.unconfirmed_email %> + + <%= user.unconfirmed_email %> + + <% else %> + + <%= user.email %> + + <% end %> + + <%= user.phone %> + <%= user.alt_phone %> + <%= time_balance(membership.account_balance.to_i) %> + <% if current_user.manages?(current_organization) %> + + <%= link_to edit_user_path(user), class: "action" do %> + <%= glyph :pencil %> + <%= t "global.edit" %> + <% end %> + + <% if membership.active? %> + <%= link_to toggle_manager_member_path(membership), class: "action", method: :put, data: { confirm: t('users.index.manage_warning', username: user.username) } do %> + <% if !!membership.manager %> + <%= glyph :arrow_down %> + <%= t "global.demote" %> + <% else %> + <%= glyph :arrow_up %> + <%= t "global.promote" %> + <% end %> + <% end %> + <% else %> + <%= link_to cancel_member_path(membership), class: "action", data: { confirm: t('.cancel_warning', user: user.username) }, method: :delete do %> + <%= glyph :ban_circle %> + <%= t("global.cancel_membership") %> + <% end %> + <% end %> + + <%= link_to toggle_active_member_path(membership), class: "action", method: :put, data: { confirm: t('users.index.active_warning', username: user.username) } do %> + <% if membership.active? %> + <%= glyph :remove %> + <%= t ".deactivate" %> + <% else %> + <%= glyph :ok %> + <%= t ".activate" %> + <% end %> + <% end %> + + <% end %> <% end %> - +<% end %> diff --git a/app/views/users/index.html.erb b/app/views/users/index.html.erb index 546e630fa..316bf2e29 100644 --- a/app/views/users/index.html.erb +++ b/app/views/users/index.html.erb @@ -1,23 +1,14 @@ -
+

<%= User.model_name.human.pluralize %> — <%= link_to current_organization.name, organization_path(current_organization) %> - <% if params[:q].present? %> - - <%= glyph :search %> - <%= params[:q] %> - - <% end %>

-
- " - type="text"> -
+ <%= search_form_for(@search, class: "navbar-form navbar-left") do |f| %> + <%= f.search_field :username_or_email_contains, class: "form-control" %> + <% end %>
-
- - - - - +
diff --git a/app/views/users/index.js.erb b/app/views/users/index.js.erb deleted file mode 100644 index 8652f5c3e..000000000 --- a/app/views/users/index.js.erb +++ /dev/null @@ -1,2 +0,0 @@ -$('table.users tbody').append("<%= j(render 'users/user_rows', users: @users) %>"); -$('table.users a[rel=next]').replaceWith("<%= link_to_next_page @users, t('.more'), remote: true, class: 'btn btn-default', params: params %>"); diff --git a/config/locales/ca.yml b/config/locales/ca.yml index 4a9a35cf8..5dec95ead 100644 --- a/config/locales/ca.yml +++ b/config/locales/ca.yml @@ -487,10 +487,10 @@ ca: give_time: Donar Temps a index: actions: Accions - active_warning_angular: Vas a canviar l'estat del compte de l'usuari {{username}} - cancel_warning_angular: Vas a eliminar del banc a l'usuari {{username}} + active_warning: Vas a canviar l'estat del compte de l'usuari %{username} + cancel_warning: Vas a eliminar del banc a l'usuari %{username} create: Crear nou usuari - manage_warning_angular: Vas a canviar els permisos de l'usuari {{username}} + manage_warning: Vas a canviar els permisos de l'usuari %{username} user_created: Usuari %{uid} %{name} guardat new: cancel: Cancel·lar diff --git a/config/locales/en.yml b/config/locales/en.yml index 31ea4fdfa..25e3338b9 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -65,7 +65,7 @@ en: identity_document: Identity Document last_sign_in_at: Last login notifications: Receive notifications - organization: + organization: phone: Phone registration_date: Registration date registration_number: User code @@ -218,20 +218,20 @@ en: registrations: destroyed: Bye! Your account was successfully cancelled. We hope to see you again soon. edit: - cancel_account: - current_password: - edit_user: - help_current_password: - help_password: - password: - password_confirmation: - unhappy: - update: + cancel_account: + current_password: + edit_user: + help_current_password: + help_password: + password: + password_confirmation: + unhappy: + update: new: - password: - password_confirmation: - sign_me_up: - sign_up: + password: + password_confirmation: + sign_me_up: + sign_up: signed_up: Welcome! You have signed up successfully. signed_up_but_inactive: You have signed up successfully. However, we could not sign you in because your account is not yet activated. signed_up_but_locked: You have signed up successfully. However, we could not sign you in because your account is locked. @@ -315,14 +315,14 @@ en: report: report_title: REPORT locales: - ar: + ar: ca: Catalan en: English es: Spanish - eu: - fr: - gl: - pt: + eu: + fr: + gl: + pt: pt-BR: Portuguese mailers_globals: footer: @@ -467,7 +467,7 @@ en: terms: accept: Accept show: - accept: + accept: transfers: computation: hour: @@ -487,10 +487,10 @@ en: give_time: Give time to index: actions: Actions - active_warning_angular: You are going to change user account status for {{username}} - cancel_warning_angular: You are going to delete account from the Time Bank for user {{username}} + active_warning: You are going to change user account status for %{username} + cancel_warning: You are going to delete account from the Time Bank for user %{username} create: Create new user - manage_warning_angular: You are going to change privileges for user {{username}} + manage_warning: You are going to change privileges for user %{username} user_created: User %{uid} %{name} saved new: cancel: Cancel @@ -524,8 +524,8 @@ en: manage_warning: You are going to change privileges for user %{user} views: pagination: - first: - last: - next: - previous: - truncate: + first: + last: + next: + previous: + truncate: diff --git a/config/locales/es.yml b/config/locales/es.yml index 63612b342..cf1d6a8df 100644 --- a/config/locales/es.yml +++ b/config/locales/es.yml @@ -65,7 +65,7 @@ es: identity_document: DNI last_sign_in_at: Fecha último login notifications: Recibir notificaciones - organization: + organization: phone: Teléfono registration_date: Fecha de alta registration_number: Código de usuario @@ -127,7 +127,7 @@ es: sing_out: Desconectar menus: offers_by_tag_link: - tags: + tags: navbar: admin: Administrar administration: Administración @@ -246,7 +246,7 @@ es: remember_me: Recordarme sign_in: Entrar user: - signed_in: + signed_in: shared: links: didnt_receive_confirmation_instructions: Volver a enviar correo de confirmación @@ -472,7 +472,7 @@ es: terms: accept: Aceptar show: - accept: + accept: transfers: computation: hour: @@ -492,10 +492,10 @@ es: give_time: Dar Tiempo a index: actions: Acciones - active_warning_angular: Va a cambiar el estado de la cuenta del usuario {{username}} - cancel_warning_angular: Va a eliminar del banco al usuario {{username}} + active_warning: Va a cambiar el estado de la cuenta del usuario %{username} + cancel_warning: Va a eliminar del banco al usuario %{username} create: Crear nuevo usuario - manage_warning_angular: Va a cambiar los privilegios del usuario {{username}} + manage_warning: Va a cambiar los privilegios del usuario %{username} user_created: Usuario %{uid} %{name} guardado new: cancel: Cancelar diff --git a/config/locales/pt-BR.yml b/config/locales/pt-BR.yml index 518f2dae2..1791758e2 100644 --- a/config/locales/pt-BR.yml +++ b/config/locales/pt-BR.yml @@ -492,10 +492,10 @@ pt-BR: give_time: Dar Tempo a index: actions: Ações - active_warning_angular: Mudará o estado da conta do usuário {{username}} - cancel_warning_angular: Eliminará o usuário do banco {{username}} + active_warning: Mudará o estado da conta do usuário %{username} + cancel_warning: Eliminará o usuário do banco %{username} create: Criar novo usuário - manage_warning_angular: Mudará os privilégios do usuário {{username}} + manage_warning: Mudará os privilégios do usuário %{username} user_created: Usuário %{uid} %{name} guardado new: cancel: Cancelar From 310db059abe5fa9599d63462559e841c7ef1d920 Mon Sep 17 00:00:00 2001 From: Victor Martinez Date: Wed, 18 Oct 2017 18:04:14 +0200 Subject: [PATCH 19/58] Remove angular traces --- app/assets/javascripts/app/app.js.coffee | 2 - app/assets/javascripts/application.js.coffee | 6 +- app/assets/javascripts/libs.js | 3 - app/assets/stylesheets/application.css.scss | 9 +- app/views/layouts/application.html.erb | 8 +- vendor/assets/javascripts/angular.min.js | 210 - .../javascripts/ng-rails-csrf.js.coffee | 17 - .../javascripts/ui-bootstrap-tpls-0.11.0.js | 4116 ----------------- 8 files changed, 6 insertions(+), 4365 deletions(-) delete mode 100644 app/assets/javascripts/app/app.js.coffee delete mode 100644 vendor/assets/javascripts/angular.min.js delete mode 100644 vendor/assets/javascripts/ng-rails-csrf.js.coffee delete mode 100644 vendor/assets/javascripts/ui-bootstrap-tpls-0.11.0.js diff --git a/app/assets/javascripts/app/app.js.coffee b/app/assets/javascripts/app/app.js.coffee deleted file mode 100644 index 3f41c7d5b..000000000 --- a/app/assets/javascripts/app/app.js.coffee +++ /dev/null @@ -1,2 +0,0 @@ -# override this in a view where the organizations are needed -angular.module('timeoverflow').value 'Organizations', [] diff --git a/app/assets/javascripts/application.js.coffee b/app/assets/javascripts/application.js.coffee index 8fcd14578..d37fff8f0 100644 --- a/app/assets/javascripts/application.js.coffee +++ b/app/assets/javascripts/application.js.coffee @@ -2,15 +2,11 @@ #= require datepicker #= require give_time #= require tags -#= require_tree ./app - -angular.module "timeoverflow", ["ng-rails-csrf", 'ui.bootstrap'] $(document).on 'click', 'a[data-popup]', (event) -> window.open($(this).attr('href'), 'popup', 'width=600,height=600') event.preventDefault() - $(document).on 'click', 'span.show-password', (event) -> if $(this).hasClass('checked') $(this).removeClass('checked'); @@ -20,4 +16,4 @@ $(document).on 'click', 'span.show-password', (event) -> $(this).addClass('checked'); $(this).prev('input').attr('type', 'text'); $(this).find('.material-icons').html("visibility_off") - event.preventDefault() \ No newline at end of file + event.preventDefault() diff --git a/app/assets/javascripts/libs.js b/app/assets/javascripts/libs.js index d54523d4c..63a46c808 100644 --- a/app/assets/javascripts/libs.js +++ b/app/assets/javascripts/libs.js @@ -6,6 +6,3 @@ //= require highcharts //= require highcharts-exporting //= require select2 -//= require angular.min -//= require ui-bootstrap-tpls-0.11.0 -//= require ng-rails-csrf diff --git a/app/assets/stylesheets/application.css.scss b/app/assets/stylesheets/application.css.scss index 73c38db59..d6bbe170c 100644 --- a/app/assets/stylesheets/application.css.scss +++ b/app/assets/stylesheets/application.css.scss @@ -27,11 +27,6 @@ $pages-anchor-hover: #8a8a8a; $features-background: #f5f5f5; $features-separator: #d8d8d8; -[ng-cloak], -.ng-cloak { - visibility: hidden !important; -} - html { font-size:62.5%; } @@ -166,7 +161,7 @@ html { font-weight: 400; height: 7.5rem; } - + .form-control:focus { border-color: none; box-shadow: none; @@ -622,7 +617,7 @@ form .checkbox input[type="checkbox"] { .material-icons { font-size: 3rem; } - + .checkbox { color: $form-login-gray-text; font-size: 1.6rem; diff --git a/app/views/layouts/application.html.erb b/app/views/layouts/application.html.erb index 71b40ce46..c9aa9882d 100644 --- a/app/views/layouts/application.html.erb +++ b/app/views/layouts/application.html.erb @@ -23,9 +23,7 @@ <%= javascript_include_tag 'libs' %> <%= javascript_include_tag 'application' %> - + <%= render 'navbar' %>
<%= render 'layouts/messages' %> @@ -37,7 +35,7 @@
-
+

@@ -60,7 +58,7 @@

-
+
diff --git a/app/views/users/give_time.html.erb b/app/views/users/give_time.html.erb deleted file mode 100644 index 2b5b143df..000000000 --- a/app/views/users/give_time.html.erb +++ /dev/null @@ -1,66 +0,0 @@ -

- - <%= t ".give_time" %> - - <%= link_to @user.member(current_organization).display_name_with_uid, user_path(@user) %> -

-<% if @offer %> -

- <%= @offer %> -

-<% end %> -<%= simple_form_for @transfer do |f| %> -
- <%= f.input :hours, - as: :integer, - input_html: { - min: 0, - max: 20, - "data-rule-either-hours-minutes-informed" => "true", - "data-rule-range" => "[0,20]" - } %> - <%= 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.input :destination, as: :hidden %> - <% if admin? %> -
- <%= f.label :source %> - <%= f.select :source, - options_for_select( - @sources.sort_by { |source| [source.accountable_type, source.accountable.try(:member_uid)] } - .map { |a| ["#{a.accountable_type == "Member" ? a.accountable.member_uid : ""} #{a.accountable_type} #{a.accountable.to_s}", a.id] }, - selected: current_user.member(current_organization).account.id - ), - {}, - id: "select2-time", - class: "form-control" %> -
- <% end %> - <% if @offer %> - <%= f.input :post, readonly: true %> - <%= f.input :post_id, as: :hidden %> - <% else %> - <%= f.input :post_id, collection: @user.member(current_organization).offers.active %> - <% end %> -
-
- <%= f.button :submit, class: "btn btn-default", style: "margin-bottom: 20px;" %> -
-<% end %> - - From b319ed9ea49f5d8584c35f9ff2d73356a93d8b1f Mon Sep 17 00:00:00 2001 From: Pau Perez Date: Thu, 4 Jan 2018 16:34:07 +0100 Subject: [PATCH 26/58] Fix doc --- app/models/transfer_sources_options.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/transfer_sources_options.rb b/app/models/transfer_sources_options.rb index 0a840af70..db93056e8 100644 --- a/app/models/transfer_sources_options.rb +++ b/app/models/transfer_sources_options.rb @@ -12,7 +12,7 @@ def initialize(sources, destination_accountable) end # Returns the collection as an Array containing pairs of