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/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..3fc377730 100644 --- a/app/controllers/members_controller.rb +++ b/app/controllers/members_controller.rb @@ -1,11 +1,36 @@ class MembersController < ApplicationController before_filter :authenticate_user! + # GET /members + # + def index + context = current_organization + .members + .includes(:account, :user) + context = context.where(active: true) unless (admin? || superadmin?) + + @memberships = context + 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 @@ -31,14 +56,54 @@ 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 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 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 68f6fb828..723a57006 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -5,20 +5,13 @@ 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 - + # GET /users/:id + # def show - @user = find_user - @member = @user.as_member_of(current_organization) - @movements = @member.movements.order("created_at DESC").page(params[:page]). - per(10) + @user = User.find_by_id(params[:id]) + # TODO: better not found management please + raise unless @user + authorize @user end def new @@ -61,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 @@ -85,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/helpers/users_helper.rb b/app/helpers/users_helper.rb index 947b95fa0..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), - 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, + id: membership.user_id, + avatar: avatar_url(membership.user), + member_uid: membership.member_uid, + 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: user_path(user), - edit_link: edit_user_path(user), + url: member_path(membership.member_uid), + 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/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 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/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 @@ +
+
+

+ <%= current_user.username %> + + <% if current_user.superadmin? %> + + <%= t ".superadmin" %> + + <% end %> + +

+
+
+ + <%= image_tag avatar_url(current_user, 160) %> + +
+
+ <%= m current_user.description %> +
    +
  • + <%= link_to account_edit_path do %> + <%= glyph :pencil %> + <%= t "global.edit" %> + <% end %> +
    • +
+
+

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

+ <% if current_user.email_if_real != "" %> +
+ <%= User.human_attribute_name(:email) %> +
+
+ <%= current_user.email_if_real %> +
+ <% end %> + <% phones = [current_user.phone, current_user.alt_phone].select(&:present?) %> + <% if phones.present? %> +
+ <%= t(".phone", count: phones.size) %> +
+
+ <%= phones.map(&:to_s).join('—') %> +
+ <% end %> +
+
+
+
+
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 @@
-<% 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_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/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 91% rename from app/views/users/_user_rows.html.erb rename to app/views/members/_members_rows.html.erb index 0f7ada7b8..4f45bcc47 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}} @@ -21,10 +21,6 @@ {{user.balance | timeBalance}} <% if current_user.manages?(current_organization) %> - - <%= glyph :pencil %> - <%= t "global.edit" %> - <%= 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/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..d0f549e2a 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: @memberships %>

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

    + <%= 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_member_path(@member.member_uid, 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/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 f5e7fa4c9..101e6579a 100644 --- a/app/views/users/show.html.erb +++ b/app/views/users/show.html.erb @@ -1,12 +1,7 @@
    -
    -
    -
    -
    -

    - <%= 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..d3f287fad 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 @@ -29,7 +32,7 @@ end end - resources :users, concerns: :accountable, except: :destroy, :path => "members" + resources :users, except: :destroy resources :transfers, only: [:create] do member do @@ -39,7 +42,7 @@ resources :documents - resources :members, only: [: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 new file mode 100644 index 000000000..5d9bc9cec --- /dev/null +++ b/spec/controllers/members_controller_spec.rb @@ -0,0 +1,137 @@ +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 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) } + + 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 + + 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 d6dc83b50..3f4741fdf 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,45 +37,96 @@ 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) + describe 'GET #show' do + context 'when the user is the same user' do + before { login(user) } - get "index" - expect(assigns(:users)).to eq([user, another_user, - admin_user, wrong_user, - empty_email_user]) + it 'assigns user to @user' do + get :show, id: user + expect(assigns(:user)).to eq(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]) + + 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("