From 3e55bc3dfeb8253a91e245d3108a9c348b81da8f Mon Sep 17 00:00:00 2001 From: Enrico Stano Date: Wed, 16 Aug 2017 17:19:52 +0200 Subject: [PATCH 1/9] Move membership behavior from User to Member --- app/controllers/application_controller.rb | 2 +- app/controllers/home_controller.rb | 2 +- app/controllers/members_controller.rb | 38 +++- app/controllers/users_controller.rb | 12 -- app/helpers/users_helper.rb | 4 +- app/views/application/_navbar.html.erb | 4 +- .../menus/_members_list_link.html.erb | 6 + .../menus/_user_list_link.html.erb | 6 - .../_members_rows.html.erb} | 2 +- app/views/{users => members}/index.html.erb | 6 +- app/views/members/show.html.erb | 168 ++++++++++++++++++ app/views/users/show.html.erb | 104 +---------- config/routes.rb | 4 +- 13 files changed, 224 insertions(+), 134 deletions(-) create mode 100644 app/views/application/menus/_members_list_link.html.erb delete mode 100644 app/views/application/menus/_user_list_link.html.erb rename app/views/{users/_user_rows.html.erb => members/_members_rows.html.erb} (98%) rename app/views/{users => members}/index.html.erb (95%) create mode 100644 app/views/members/show.html.erb diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 2b934011e..923b2a603 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -47,7 +47,7 @@ def store_location def after_sign_in_path_for(user) if user.members.present? - users_path + members_path else page_path("about") end diff --git a/app/controllers/home_controller.rb b/app/controllers/home_controller.rb index d257c39e4..6da7e5d10 100644 --- a/app/controllers/home_controller.rb +++ b/app/controllers/home_controller.rb @@ -1,5 +1,5 @@ class HomeController < ApplicationController def index - redirect_to :users if current_user + redirect_to :members if current_user end end diff --git a/app/controllers/members_controller.rb b/app/controllers/members_controller.rb index c88ea52df..35f934b08 100644 --- a/app/controllers/members_controller.rb +++ b/app/controllers/members_controller.rb @@ -1,11 +1,38 @@ class MembersController < ApplicationController before_filter :authenticate_user! + # GET /members + # + def index + @users = current_organization.users + @memberships = current_organization + .members + .where(user_id: @users.map(&:id)) + .includes(:account) + .each_with_object({}) do |mem, ob| + ob[mem.user_id] = mem + end + end + + # GET /members/:member_uid + # + def show + find_member + @user = @member.user + @movements = @member + .movements + .order('created_at DESC') + .page(params[:page]) + .per(10) + end + + # DELETE /members/:member_uid + # def destroy find_member toggle_active_posts @member.destroy - redirect_to users_path + redirect_to members_path end def toggle_manager @@ -33,8 +60,15 @@ def toggle_active private + # TODO: rely on organization scope instead of current_organization def find_member - @member ||= current_organization.members.find(params[:id]) + @member ||= Member.where( + organization_id: current_organization.id, + member_uid: params[:member_uid] + ).first + + # TODO: better not found management please + raise unless @member end def toggle_active_posts diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 68f6fb828..2ce069a70 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -5,20 +5,8 @@ def scoped_users current_organization.users end - def index - @users = scoped_users - @memberships = current_organization.members. - where(user_id: @users.map(&:id)). - includes(:account).each_with_object({}) do |mem, ob| - ob[mem.user_id] = mem - end - end - def show @user = find_user - @member = @user.as_member_of(current_organization) - @movements = @member.movements.order("created_at DESC").page(params[:page]). - per(10) end def new diff --git a/app/helpers/users_helper.rb b/app/helpers/users_helper.rb index 947b95fa0..34af1df0b 100644 --- a/app/helpers/users_helper.rb +++ b/app/helpers/users_helper.rb @@ -7,7 +7,7 @@ def users_as_json { id: user.id, avatar: avatar_url(user), - member_id: membership.member_uid, + member_uid: membership.member_uid, username: user.username, email: user.email_if_real, unconfirmed_email: user.unconfirmed_email, @@ -15,7 +15,7 @@ def users_as_json alt_phone: user.alt_phone, balance: membership.account_balance.to_i, - url: user_path(user), + url: member_path(membership.member_uid), edit_link: edit_user_path(user), cancel_link: cancel_member_path(membership), toggle_manager_link: toggle_manager_member_path(membership), diff --git a/app/views/application/_navbar.html.erb b/app/views/application/_navbar.html.erb index f6520881d..283aba0ea 100644 --- a/app/views/application/_navbar.html.erb +++ b/app/views/application/_navbar.html.erb @@ -48,7 +48,7 @@
    - <%= render 'application/menus/user_list_link' %> + <%= render 'application/menus/members_list_link' %> <%= render 'application/menus/offers_dashboard_link' %> <%= render 'application/menus/inquiries_list_link' %> <%= render 'application/menus/offers_by_tag_link' %> @@ -63,4 +63,4 @@
-<% end %> \ No newline at end of file +<% end %> diff --git a/app/views/application/menus/_members_list_link.html.erb b/app/views/application/menus/_members_list_link.html.erb new file mode 100644 index 000000000..ee8c9ccbb --- /dev/null +++ b/app/views/application/menus/_members_list_link.html.erb @@ -0,0 +1,6 @@ +
  • "> + <%= link_to members_path do %> + <%= glyph :user %> + <%= Member.model_name.human.pluralize %> + <% end %> +
    • diff --git a/app/views/application/menus/_user_list_link.html.erb b/app/views/application/menus/_user_list_link.html.erb deleted file mode 100644 index e3bf2c45e..000000000 --- a/app/views/application/menus/_user_list_link.html.erb +++ /dev/null @@ -1,6 +0,0 @@ -
  • "> - <%= link_to users_path do %> - <%= glyph :user %> - <%= User.model_name.human(count: :many) %> - <% end %> -
    • diff --git a/app/views/users/_user_rows.html.erb b/app/views/members/_members_rows.html.erb similarity index 98% rename from app/views/users/_user_rows.html.erb rename to app/views/members/_members_rows.html.erb index 0f7ada7b8..5abdc6f1c 100644 --- a/app/views/users/_user_rows.html.erb +++ b/app/views/members/_members_rows.html.erb @@ -3,7 +3,7 @@ - {{user.member_id}} + {{user.member_uid}} {{user.username}} diff --git a/app/views/users/index.html.erb b/app/views/members/index.html.erb similarity index 95% rename from app/views/users/index.html.erb rename to app/views/members/index.html.erb index 546e630fa..c1e652b91 100644 --- a/app/views/users/index.html.erb +++ b/app/views/members/index.html.erb @@ -1,6 +1,6 @@

    - <%= User.model_name.human.pluralize %> + <%= Member.model_name.human.pluralize %> — <%= link_to current_organization.name, organization_path(current_organization) %> @@ -23,7 +23,7 @@
  • - <%= t "helpers.submit.create", model: User.model_name.human %> + <%= t "helpers.submit.create", model: Member.model_name.human %>
    • <% end %> @@ -78,7 +78,7 @@ - <%= render "user_rows", users: @users %> + <%= render "members_rows", users: @users %>

    diff --git a/app/views/members/show.html.erb b/app/views/members/show.html.erb new file mode 100644 index 000000000..d4fc37d33 --- /dev/null +++ b/app/views/members/show.html.erb @@ -0,0 +1,168 @@ +
    +
    +

    + + <% unless @member.active %> + <%= t ".inactive" %> + <% end %> + + <%= @member.display_name_with_uid %> + + <% if @user.superadmin? %> + + <%= t ".superadmin" %> + + <% end %> + +

    +
    +
    + <% if @user == current_user %> + + <%= image_tag avatar_url(@user, 160) %> + + <% else %> + <%= image_tag avatar_url(@user, 160) %> + <% end %> +
    +
    + <%= m @user.description %> +
      + <% if @user == current_user %> +
    • + <%= link_to account_edit_path do %> + <%= glyph :pencil %> + <%= t "global.edit" %> + <% end %> +
      • + <% end %> + <% if admin? || @user != current_user %> +
    • + <%= link_to give_time_user_path(@user) do %> + <%= glyph :time %> + <%= t "global.give_time" %> + <% end %> +
      • + <% end %> +
    +
    +

    + <%= t "global.contact_details" %> +

    + <% if @user.email_if_real != "" %> +
    + <%= User.human_attribute_name(:email) %> +
    +
    + <%= @user.email_if_real %> +
    + <% end %> + <% phones = [@user.phone, @user.alt_phone].select(&:present?) %> + <% if phones.present? %> +
    + <%= t(".phone", count: phones.size) %> +
    +
    + <%= phones.map(&:to_s).join('—') %> +
    + <% end %> +
    +
    +
    +
    +
    +
    +
    +
    +
    +

    + <%= Offer.model_name.human(count: :many) %> + <% if @user == current_user %> + + <%= glyph :plus %> + + <% end %> +

    +
    + <% @member.offers.active.each do |post| %> +
    +
    + <%= link_to post, post %> +
    +
    + <%= strip_tags(post.rendered_description.to_html) %> +
    +
    + <% if @user != current_user %> + <%= link_to give_time_user_path(@user, offer: post.id) do %> + <%= glyph :time %> + <% end %> + <% end %> +
    +
    + <% end %> + +
    +
    +
    +
    +
    +

    + <%= Inquiry.model_name.human(count: :many) %> + <% if @user == current_user %> + + <%= glyph :plus %> + + <% end %> +

    +
    + <% @member.inquiries.active.each do |post| %> +
    +
    + <%= link_to post, post %> +
    +
    + <%= strip_tags(post.rendered_description.to_html) %> +
    +
    + <% end %> + +
    +
    +
    +
    +
    +
    +
    +

    + <%= t(".accounts") %> +

    +
    +
    + <% if @member.manager %> +

    + ADMIN +

    + <% end %> +

    + + <%= t(".created_at") %> + + <%= @member.entry_date ? l(@member.entry_date, format: :long) : mdash %> +
    + + <%= t(".user_no") %> + + <%= @member.member_uid || mdash %> +
    + + <%= t(".balance") %> + <%= seconds_to_hm(@member.account.try(:balance) || mdash) %> + +

    +
    +
    +
    +
    + +<%= render 'shared/movements' %> diff --git a/app/views/users/show.html.erb b/app/views/users/show.html.erb index f5e7fa4c9..4dda6b1b2 100644 --- a/app/views/users/show.html.erb +++ b/app/views/users/show.html.erb @@ -1,12 +1,7 @@

    - - <% unless @member.active %> - <%= t ".inactive" %> - <% end %> - - <%= @member.display_name_with_uid %> + <%= @user.username %> <% if @user.superadmin? %> @@ -30,7 +25,7 @@
      <% if @user == current_user %>
    • - <%= link_to edit_user_path(@user) do %> + <%= link_to account_edit_path do %> <%= glyph :pencil %> <%= t "global.edit" %> <% end %> @@ -71,98 +66,3 @@

    -
    -
    -
    -
    -

    - <%= Offer.model_name.human(count: :many) %> - <% if @user == current_user %> - - <%= glyph :plus %> - - <% end %> -

    -
    - <% @member.offers.active.each do |post| %> -
    -
    - <%= link_to post, post %> -
    -
    - <%= strip_tags(post.rendered_description.to_html) %> -
    -
    - <% if @user != current_user %> - <%= link_to give_time_user_path(@user, offer: post.id) do %> - <%= glyph :time %> - <% end %> - <% end %> -
    -
    - <% end %> - -
    -
    -
    -
    -
    -

    - <%= Inquiry.model_name.human(count: :many) %> - <% if @user == current_user %> - - <%= glyph :plus %> - - <% end %> -

    -
    - <% @member.inquiries.active.each do |post| %> -
    -
    - <%= link_to post, post %> -
    -
    - <%= strip_tags(post.rendered_description.to_html) %> -
    -
    - <% end %> - -
    -
    -
    -
    -
    -
    -
    -

    - <%= t(".accounts") %> -

    -
    -
    - <% if @member.manager %> -

    - ADMIN -

    - <% end %> -

    - - <%= t(".created_at") %> - - <%= @member.entry_date ? l(@member.entry_date, format: :long) : mdash %> -
    - - <%= t(".user_no") %> - - <%= @member.member_uid || mdash %> -
    - - <%= t(".balance") %> - <%= seconds_to_hm(@member.account.try(:balance) || mdash) %> - -

    -
    -
    -
    -
    - -<%= render 'shared/movements' %> diff --git a/config/routes.rb b/config/routes.rb index e03841279..4b6e0b57e 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -29,7 +29,7 @@ end end - resources :users, concerns: :accountable, except: :destroy, :path => "members" + resources :users, concerns: :accountable, except: :destroy resources :transfers, only: [:create] do member do @@ -39,7 +39,7 @@ resources :documents - resources :members, only: [:destroy] do + resources :members, param: :member_uid, only: [:index, :show, :destroy] do member do put :toggle_manager put :toggle_active From ca60947d4132a846943d8ba7b34ce009d9cdc379 Mon Sep 17 00:00:00 2001 From: Enrico Stano Date: Wed, 16 Aug 2017 17:22:35 +0200 Subject: [PATCH 2/9] Add /account resource --- app/controllers/account_controller.rb | 12 +++++ app/views/account/edit.html.erb | 6 +++ app/views/account/show.html.erb | 54 +++++++++++++++++++ .../menus/_user_admin_menu.html.erb | 2 +- config/routes.rb | 3 ++ 5 files changed, 76 insertions(+), 1 deletion(-) create mode 100644 app/controllers/account_controller.rb create mode 100644 app/views/account/edit.html.erb create mode 100644 app/views/account/show.html.erb diff --git a/app/controllers/account_controller.rb b/app/controllers/account_controller.rb new file mode 100644 index 000000000..8e49a49f6 --- /dev/null +++ b/app/controllers/account_controller.rb @@ -0,0 +1,12 @@ +class AccountController < ApplicationController + # GET /account + # + def show + end + + # GET /account/edit + # + def edit + @user = current_user + end +end diff --git a/app/views/account/edit.html.erb b/app/views/account/edit.html.erb new file mode 100644 index 000000000..8b9133d95 --- /dev/null +++ b/app/views/account/edit.html.erb @@ -0,0 +1,6 @@ +

    + <%= @user.username %> + <%= t ".edit_user" %> +

    + +<%= render "users/form" %> diff --git a/app/views/account/show.html.erb b/app/views/account/show.html.erb new file mode 100644 index 000000000..5fed0574c --- /dev/null +++ b/app/views/account/show.html.erb @@ -0,0 +1,54 @@ + diff --git a/app/views/application/menus/_user_admin_menu.html.erb b/app/views/application/menus/_user_admin_menu.html.erb index 58f394025..d46a7697c 100644 --- a/app/views/application/menus/_user_admin_menu.html.erb +++ b/app/views/application/menus/_user_admin_menu.html.erb @@ -25,7 +25,7 @@ <% end %>
  • - <%= link_to current_user do %> + <%= link_to account_path do %> <%= glyph :user %> <%= t "layouts.application.edit_profile" %> <% end %> diff --git a/config/routes.rb b/config/routes.rb index 4b6e0b57e..e6a32958f 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -11,6 +11,9 @@ get "global/switch_lang", as: :switch_lang + get :account, controller: :account, action: :show + get 'account/edit', controller: :account, action: :edit + resources :offers do collection do get :dashboard From 61ead411cc187a4a4f5f71a9f5b8d93c67ee47ab Mon Sep 17 00:00:00 2001 From: Enrico Stano Date: Tue, 22 Aug 2017 18:18:00 +0200 Subject: [PATCH 3/9] Shows user only to same user and to admin of shared organizations --- app/controllers/users_controller.rb | 6 ++- app/policies/user_policy.rb | 8 ++++ spec/controllers/users_controller_spec.rb | 47 ++++++++++++++++------- 3 files changed, 47 insertions(+), 14 deletions(-) diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 2ce069a70..35e3e0e34 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -5,8 +5,12 @@ def scoped_users current_organization.users end + # GET /users/:id + # def show - @user = find_user + @user = User.find_by_id(params[:id]) + raise unless @user + authorize @user end def new diff --git a/app/policies/user_policy.rb b/app/policies/user_policy.rb index a0b6b3739..504ce20be 100644 --- a/app/policies/user_policy.rb +++ b/app/policies/user_policy.rb @@ -3,6 +3,14 @@ def new? user.admins?(organization) end + def show? + return true if user.id == record.id + + record.organizations.any? do |org| + user.admins?(org) + end + end + def create? user.admins?(organization) end diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index d6dc83b50..068194447 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -60,23 +60,44 @@ end end - describe "GET #show" do - context "with valid params" do - context "with a normal logged user" do - it "assigns the requested user to @user" do - login(user) + describe 'GET #show' do + context 'when the user is the same user' do + before { login(user) } - get "show", id: user.id - expect(assigns(:user)).to eq(user) - end + it 'assigns user to @user' do + get :show, id: user.id + expect(assigns(:user)).to eq(user) end - context "with an admin logged user" do - it "assigns the requested user to @user" do - login(admin_user) + end - get "show", id: user.id - expect(assigns(:user)).to eq(user) + context 'when the user is not the same' do + before { login(user) } + + context 'and is not a member of the same organization' do + let(:other_organization) { Fabricate(:organization) } + let(:other_admin) do + Fabricate( + :member, + organization: test_organization, + manager: true + ) end + let(:other_user) { other_admin.user } + + it 'redirects to root path' do + get :show, id: other_user + + expect(response).to redirect_to(root_path) + end + end + end + + context "with an admin logged user" do + it "assigns the requested user to @user" do + login(admin_user) + + get "show", id: user.id + expect(assigns(:user)).to eq(user) end end end From 9e8ebb269ef5c79936844dae1165bced880c39f3 Mon Sep 17 00:00:00 2001 From: Enrico Stano Date: Wed, 23 Aug 2017 18:44:22 +0200 Subject: [PATCH 4/9] UsersController#show specs++ --- spec/controllers/users_controller_spec.rb | 72 ++++++++++++++++++++--- 1 file changed, 65 insertions(+), 7 deletions(-) diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index 068194447..97ebd3935 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -65,9 +65,30 @@ before { login(user) } it 'assigns user to @user' do - get :show, id: user.id + get :show, id: user expect(assigns(:user)).to eq(user) end + + it 'renders a page with the user username' do + get :show, id: user + expect(response.body).to include(user.username) + end + + it 'renders a page with the user avatar as link to Gravatar' do + get :show, id: user + expect(response.body).to include("") + expect(response.body).to include("") + end + end end end end From 09e406c33ddc025bf28fbbbade4151cf4d4de261 Mon Sep 17 00:00:00 2001 From: Enrico Stano Date: Fri, 15 Sep 2017 20:19:58 +0200 Subject: [PATCH 5/9] MembersController #index action --- app/controllers/members_controller.rb | 12 ++-- app/helpers/users_helper.rb | 24 ++++--- app/views/members/index.html.erb | 2 +- spec/controllers/members_controller_spec.rb | 69 +++++++++++++++++++++ spec/controllers/users_controller_spec.rb | 25 +------- 5 files changed, 87 insertions(+), 45 deletions(-) create mode 100644 spec/controllers/members_controller_spec.rb diff --git a/app/controllers/members_controller.rb b/app/controllers/members_controller.rb index 35f934b08..2ea9c0384 100644 --- a/app/controllers/members_controller.rb +++ b/app/controllers/members_controller.rb @@ -4,14 +4,12 @@ class MembersController < ApplicationController # GET /members # def index - @users = current_organization.users - @memberships = current_organization + context = current_organization .members - .where(user_id: @users.map(&:id)) - .includes(:account) - .each_with_object({}) do |mem, ob| - ob[mem.user_id] = mem - end + .includes(:account, :user) + context = context.where(active: true) unless (admin? || superadmin?) + + @memberships = context end # GET /members/:member_uid diff --git a/app/helpers/users_helper.rb b/app/helpers/users_helper.rb index 34af1df0b..757cedfc8 100644 --- a/app/helpers/users_helper.rb +++ b/app/helpers/users_helper.rb @@ -1,28 +1,26 @@ module UsersHelper - # TODO refactor or eliminate - poosibly the second. + # TODO refactor or eliminate - possibly the second. def users_as_json - @users = (admin? || superadmin?) ? @users : @users.actives - @users.map do |user| - membership = @memberships[user.id] + @memberships.map do |membership| { - id: user.id, - avatar: avatar_url(user), + id: membership.user_id, + avatar: avatar_url(membership.user), member_uid: membership.member_uid, - username: user.username, - email: user.email_if_real, - unconfirmed_email: user.unconfirmed_email, - phone: user.phone, - alt_phone: user.alt_phone, + username: membership.user.username, + email: membership.user.email_if_real, + unconfirmed_email: membership.user.unconfirmed_email, + phone: membership.user.phone, + alt_phone: membership.user.alt_phone, balance: membership.account_balance.to_i, url: member_path(membership.member_uid), - edit_link: edit_user_path(user), + edit_link: edit_user_path(membership.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? + valid_email: membership.user.has_valid_email? } end.to_json.html_safe end diff --git a/app/views/members/index.html.erb b/app/views/members/index.html.erb index c1e652b91..d0f549e2a 100644 --- a/app/views/members/index.html.erb +++ b/app/views/members/index.html.erb @@ -78,7 +78,7 @@ - <%= render "members_rows", users: @users %> + <%= render "members_rows", users: @memberships %> diff --git a/spec/controllers/members_controller_spec.rb b/spec/controllers/members_controller_spec.rb new file mode 100644 index 000000000..aa8bef51e --- /dev/null +++ b/spec/controllers/members_controller_spec.rb @@ -0,0 +1,69 @@ +require 'spec_helper' + +describe MembersController do + let(:organization) { Fabricate(:organization) } + let!(:member_admin) do + Fabricate( + :member, + organization: organization, + manager: true + ) + end + let!(:member) do + Fabricate( + :member, + organization: organization, + manager: false + ) + end + let!(:another_member) do + Fabricate( + :member, + organization: organization, + manager: false + ) + end + let!(:inactive_member) do + Fabricate( + :member, + organization: organization, + manager: false, + active: false + ) + end + let(:user) { member.user } + let(:admin) { member_admin.user } + + describe '#index' do + context 'when the logged user is not an admin' do + before { login(user) } + + it 'populates an array of active members' do + get :index + expect(assigns(:memberships)).to eq( + [ + member_admin, + member, + another_member + ] + ) + end + end + + context 'when the logged user is an admin' do + before { login(admin) } + + it 'populates a collection of all members' do + get :index + expect(assigns(:memberships)).to eq( + [ + member_admin, + member, + another_member, + inactive_member + ] + ) + end + end + end +end diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index 97ebd3935..823953b58 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -1,4 +1,4 @@ -require "spec_helper" +require 'spec_helper' describe UsersController do let (:test_organization) { Fabricate(:organization) } @@ -37,29 +37,6 @@ include_context "stub browser locale" before { set_browser_locale("ca") } - describe "GET #index" do - context "with an normal logged user" do - it "populates and array of users" do - login(user) - - get "index" - expect(assigns(:users)).to eq([user, another_user, - admin_user, wrong_user, - empty_email_user]) - end - end - context "with an admin logged user" do - it "populates and array of users" do - login(admin_user) - - get "index" - expect(assigns(:users)).to eq([user, another_user, - admin_user, wrong_user, - empty_email_user]) - end - end - end - describe 'GET #show' do context 'when the user is the same user' do before { login(user) } From d1d73afb7322e43b19b1020061ddfc31d9441856 Mon Sep 17 00:00:00 2001 From: Enrico Stano Date: Mon, 18 Sep 2017 21:08:31 +0200 Subject: [PATCH 6/9] Add some TODO --- app/controllers/users_controller.rb | 1 + app/models/post.rb | 3 +++ 2 files changed, 4 insertions(+) diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 35e3e0e34..cedbfb54a 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -9,6 +9,7 @@ def scoped_users # def show @user = User.find_by_id(params[:id]) + # TODO: better not found management please raise unless @user authorize @user end diff --git a/app/models/post.rb b/app/models/post.rb index e8b2ee624..25d749d45 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -36,13 +36,16 @@ def self.inherited(child) end end + # TODO: what is this? Can we delete it? attr_reader :member_id belongs_to :category belongs_to :user belongs_to :organization + # TODO: what is this? belongs_to :publisher, class_name: "User", foreign_key: "publisher_id" + # TODO: what is this? Can we delete it? # belongs_to :member, class_name: "Member", foreign_key: "user_id" has_many :user_members, class_name: "Member", through: :user, source: :members has_many :transfers From e35e961e3596d82f4a0a242a95b7e78370dbf9ac Mon Sep 17 00:00:00 2001 From: Enrico Stano Date: Mon, 18 Sep 2017 21:10:17 +0200 Subject: [PATCH 7/9] Add #index spec for unauthorized --- spec/controllers/members_controller_spec.rb | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/spec/controllers/members_controller_spec.rb b/spec/controllers/members_controller_spec.rb index aa8bef51e..9f782fdf6 100644 --- a/spec/controllers/members_controller_spec.rb +++ b/spec/controllers/members_controller_spec.rb @@ -35,6 +35,14 @@ let(:admin) { member_admin.user } describe '#index' do + context 'when the user is not logged in' do + it 'responds with a redirect' do + get :index + + expect(response.status).to eq(302) + end + end + context 'when the logged user is not an admin' do before { login(user) } From ac5311f7b0548b6d4fde0ad69d36b2ef43232eb1 Mon Sep 17 00:00:00 2001 From: Enrico Stano Date: Mon, 18 Sep 2017 21:14:05 +0200 Subject: [PATCH 8/9] Move #give_time from user to member --- app/controllers/members_controller.rb | 33 ++++++++++ app/controllers/posts_controller.rb | 6 +- app/controllers/users_controller.rb | 28 --------- .../{users => members}/give_time.html.erb | 4 +- app/views/members/show.html.erb | 4 +- app/views/offers/show.html.erb | 2 +- app/views/users/show.html.erb | 8 --- config/routes.rb | 4 +- spec/controllers/members_controller_spec.rb | 60 +++++++++++++++++++ spec/controllers/users_controller_spec.rb | 5 -- 10 files changed, 105 insertions(+), 49 deletions(-) rename app/views/{users => members}/give_time.html.erb (91%) diff --git a/app/controllers/members_controller.rb b/app/controllers/members_controller.rb index 2ea9c0384..3fc377730 100644 --- a/app/controllers/members_controller.rb +++ b/app/controllers/members_controller.rb @@ -56,6 +56,20 @@ def toggle_active end end + # TODO: move to service and probably different controller + def give_time + find_member + @destination = @member.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 # TODO: rely on organization scope instead of current_organization @@ -73,4 +87,23 @@ def toggle_active_posts current_organization.posts.where(user_id: @member.user_id). each { |post| post.update_attributes(active: false) } end + + # TODO: move to service and probably different controller + def find_transfer_offer + current_organization.offers. + find(params[:offer]) if params[:offer].present? + end + + # TODO: move to service and probably different controller + def find_transfer_source + current_user.members. + find_by(organization: current_organization).account.id + end + + # TODO: move to service and probably different controller + 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/posts_controller.rb b/app/controllers/posts_controller.rb index 08c47942c..fcd7c31b0 100644 --- a/app/controllers/posts_controller.rb +++ b/app/controllers/posts_controller.rb @@ -60,7 +60,11 @@ def show else model.all.active.of_active_members end - post = scope.find params[:id] + post = scope.find(params[:id]) + @member = Member.where( + organization: post.organization, + user: post.user + ) instance_variable_set("@#{resource}", post) end diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index cedbfb54a..723a57006 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -54,18 +54,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 @@ -78,22 +66,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 diff --git a/app/views/users/give_time.html.erb b/app/views/members/give_time.html.erb similarity index 91% rename from app/views/users/give_time.html.erb rename to app/views/members/give_time.html.erb index 2b5b143df..0e5d7f9c4 100644 --- a/app/views/users/give_time.html.erb +++ b/app/views/members/give_time.html.erb @@ -2,7 +2,7 @@ <%= t ".give_time" %> - <%= link_to @user.member(current_organization).display_name_with_uid, user_path(@user) %> + <%= link_to @member.display_name_with_uid, member_path(@member.member_uid) %> <% 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: @member.offers.active %> <% end %>
    diff --git a/app/views/members/show.html.erb b/app/views/members/show.html.erb index d4fc37d33..ca26e6698 100644 --- a/app/views/members/show.html.erb +++ b/app/views/members/show.html.erb @@ -38,7 +38,7 @@ <% end %> <% if admin? || @user != current_user %>
  • - <%= link_to give_time_user_path(@user) do %> + <%= link_to give_time_member_path(@member.member_uid) do %> <%= glyph :time %> <%= t "global.give_time" %> <% end %> @@ -94,7 +94,7 @@
    • <% if @user != current_user %> - <%= link_to give_time_user_path(@user, offer: post.id) do %> + <%= link_to give_time_member_path(@member.member_uid, offer: post.id) do %> <%= glyph :time %> <% end %> <% end %> diff --git a/app/views/offers/show.html.erb b/app/views/offers/show.html.erb index 95d9e3a98..7a76503e9 100644 --- a/app/views/offers/show.html.erb +++ b/app/views/offers/show.html.erb @@ -12,7 +12,7 @@ <% end %> <% end %> <% if current_user and @offer.user != current_user %> - <%= link_to give_time_user_path(@offer.user, offer: @offer.id), + <%= link_to give_time_member_path(@member.member_uid, offer: @offer.id), class: "btn btn-success" do %> <%= glyph :time %> <%= t ".give_time_for" %> diff --git a/app/views/users/show.html.erb b/app/views/users/show.html.erb index 4dda6b1b2..101e6579a 100644 --- a/app/views/users/show.html.erb +++ b/app/views/users/show.html.erb @@ -31,14 +31,6 @@ <% end %>

    • <% end %> - <% if admin? || @user != current_user %> -
  • - <%= link_to give_time_user_path(@user) do %> - <%= glyph :time %> - <%= t "global.give_time" %> - <% end %> -
    • - <% end %>

    diff --git a/config/routes.rb b/config/routes.rb index e6a32958f..d3f287fad 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -32,7 +32,7 @@ end end - resources :users, concerns: :accountable, except: :destroy + resources :users, except: :destroy resources :transfers, only: [:create] do member do @@ -42,7 +42,7 @@ resources :documents - resources :members, param: :member_uid, only: [:index, :show, :destroy] do + resources :members, param: :member_uid, concerns: :accountable, only: [:index, :show, :destroy] do member do put :toggle_manager put :toggle_active diff --git a/spec/controllers/members_controller_spec.rb b/spec/controllers/members_controller_spec.rb index 9f782fdf6..5d9bc9cec 100644 --- a/spec/controllers/members_controller_spec.rb +++ b/spec/controllers/members_controller_spec.rb @@ -74,4 +74,64 @@ end end end + + describe 'GET #show' do + context 'when the user is not logged in' do + it 'responds with a redirect' do + get :show, member_uid: member.member_uid + + expect(response.status).to eq(302) + end + end + + context 'when the user is logged in' do + context 'as an admin' do + before { login(admin) } + + it 'renders a page with a link to edit user\'s details' do + get :show, member_uid: member.member_uid + + expect(response.body).to include("") + end + + it 'renders a page with a link to give time to the member' do + get :show, member_uid: member.member_uid + + expect(response.body).to include("") + end + end + + context 'as a member' do + before { login(user) } + + context 'and the member is associated to the current user' do + it 'renders a page with a link to edit user\'s details' do + get :show, member_uid: member.member_uid + + expect(response.body).to include("") + end + + it 'renders a page without a link to give time to the member' do + get :show, member_uid: member.member_uid + + expect(response.body).to_not include("") + end + end + + context 'and the member is not associated to the current user' do + it 'renders a page without a link to edit user\'s details' do + get :show, member_uid: member_admin.member_uid + + expect(response.body).to_not include("") + end + + it 'renders a page with a link to give time to the member' do + get :show, member_uid: member_admin.member_uid + + expect(response.body).to include("") + end + end + end + end + end end diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index 823953b58..3f4741fdf 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -127,11 +127,6 @@ get :show, id: user expect(response.body).to include("") - end end end end From 5c096290442a6f34e0aa7e6b0ab49fdb2a57c6f3 Mon Sep 17 00:00:00 2001 From: Enrico Stano Date: Mon, 18 Sep 2017 21:44:25 +0200 Subject: [PATCH 9/9] Remove edit user from member row --- app/views/members/_members_rows.html.erb | 4 ---- app/views/members/show.html.erb | 4 ++-- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/app/views/members/_members_rows.html.erb b/app/views/members/_members_rows.html.erb index 5abdc6f1c..4f45bcc47 100644 --- a/app/views/members/_members_rows.html.erb +++ b/app/views/members/_members_rows.html.erb @@ -21,10 +21,6 @@ {{user.balance | timeBalance}} <% if current_user.manages?(current_organization) %> - - <%= glyph :pencil %> - <%= t "global.edit" %> - <%= m @user.description %>
      - <% if @user == current_user %> + <% if admin? || @user == current_user %>
    • - <%= link_to account_edit_path do %> + <%= link_to edit_user_path(@user) do %> <%= glyph :pencil %> <%= t "global.edit" %> <% end %>