diff --git a/app/abilities/ability.rb b/app/abilities/ability.rb index 195f1be4ac9..09027998047 100644 --- a/app/abilities/ability.rb +++ b/app/abilities/ability.rb @@ -68,7 +68,7 @@ def initialize(user) can [:edit, :update], Community, user_is_community_organizer can [:edit, :create, :destroy, :new, :update], CommunityLink, { :community => user_is_community_organizer } can [:create, :destroy], CommunityMember, { :user_id => user.id } - can [:edit, :update], CommunityMember, { :community => user_is_community_organizer } + can [:destroy, :edit, :update], CommunityMember, { :community => user_is_community_organizer } if user.moderator? can [:hide, :hidecomment], DiaryEntry diff --git a/app/controllers/community_members_controller.rb b/app/controllers/community_members_controller.rb index eb05d7aca0d..3a39ae8bdcf 100644 --- a/app/controllers/community_members_controller.rb +++ b/app/controllers/community_members_controller.rb @@ -1,7 +1,7 @@ class CommunityMembersController < ApplicationController layout "site" before_action :authorize_web - before_action :set_community_member, :only => [:edit, :update] + before_action :set_community_member, :only => [:destroy, :edit, :update] load_and_authorize_resource :except => [:create, :new] authorize_resource @@ -40,6 +40,18 @@ def update end end + def destroy + issues = @community_member.can_be_deleted + if issues.empty? && @community_member.destroy + redirect_to @community_member.community, :notice => t(".success") + else + issues = issues.map { |i| t("activerecord.errors.models.community_member.#{i}") } + issues = issues.to_sentence.capitalize + flash[:error] = "#{t('.failure')} #{issues}." + redirect_to community_community_members_path(@community_member.community) + end + end + private def set_community_member diff --git a/app/models/community.rb b/app/models/community.rb index 7b12d46c522..2566dabdbc5 100644 --- a/app/models/community.rb +++ b/app/models/community.rb @@ -26,7 +26,7 @@ class Community < ApplicationRecord friendly_id :name, :use => :slugged # Organizers before members, a tad hacky, but works for now. - has_many :community_members, -> { order(:role => :desc) }, :inverse_of => :community + has_many :community_members, -> { order(:user_id) }, :inverse_of => :community has_many :users, :through => :community_members # TODO: counter_cache belongs_to :leader, :class_name => "User" diff --git a/app/models/community_member.rb b/app/models/community_member.rb index 7b8811a81ec..51e6eaed7a5 100644 --- a/app/models/community_member.rb +++ b/app/models/community_member.rb @@ -26,9 +26,21 @@ module Roles belongs_to :community belongs_to :user + scope :organizers, -> { where(:role => Roles::ORGANIZER) } + scope :members, -> { where(:role => Roles::MEMBER) } + validates :community, :associated => true validates :user, :associated => true validates :role, :inclusion => { :in => Roles::ALL_ROLES } # TODO: validate uniqueness of user's role in each community. + + # We assume this user already belongs to this community. + def can_be_deleted + issues = [] + # The user may also be an organizer under a separate membership. + issues.append(:is_organizer) if CommunityMember.exists?(:community_id => community_id, :user_id => user_id, :role => Roles::ORGANIZER) + + issues + end end diff --git a/app/views/communities/show.html.erb b/app/views/communities/show.html.erb index 99c679337f0..18c80f043e4 100644 --- a/app/views/communities/show.html.erb +++ b/app/views/communities/show.html.erb @@ -59,11 +59,17 @@ <% end %>
- <% if current_user && ! @community.member?(current_user) %> - <%= bootstrap_form_for @current_user_membership do |form| %> - <%= form.hidden_field(:community_id) %> - <%= form.hidden_field(:user_id) %> - <%= form.primary t(".join.action") %> + <% if current_user %> + <% if ! @community.member?(current_user) %> + <%= bootstrap_form_for @current_user_membership do |form| %> + <%= form.hidden_field(:community_id) %> + <%= form.hidden_field(:user_id) %> + <%= form.primary t(".join.action") %> + <% end %> + <% else %> + <%= bootstrap_form_for @current_user_membership, :method => :delete do |form| %> + <%= form.primary t(".leave.action"), :data => { :confirm => t(".leave.confirm", :name => @community.name) } %> + <% end %> <% end %> <% else %>
diff --git a/app/views/community_members/index.html.erb b/app/views/community_members/index.html.erb index ff2876ff736..5d2d7f2e809 100644 --- a/app/views/community_members/index.html.erb +++ b/app/views/community_members/index.html.erb @@ -25,6 +25,11 @@ <%= form.primary t(".edit") %> <% end %>
+
+ <%= bootstrap_form_for membership, :method => :delete, :url => community_member_path(membership) do |form| %> + <%= form.submit t(".remove") %> + <% end %> +
<% end %>
diff --git a/config/locales/en.yml b/config/locales/en.yml index cf2546e53e7..7110b3d955e 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -60,11 +60,15 @@ en: invalid_email_address: does not appear to be a valid e-mail address email_address_not_routable: is not routable url: "is not a valid secure URL" + models: + community_member: + is_organizer: "you are an organizer of this community" # Translates all the model names, which is used in error handling on the website models: acl: "Access Control List" changeset: "Changeset" changeset_tag: "Changeset Tag" + community: "Community" country: "Country" diary_comment: "Diary Comment" diary_entry: "Diary Entry" @@ -531,6 +535,9 @@ en: join: action: "Join" confirm: "Join %{name}?" + leave: + action: "Leave" + confirm: "Are you sure you want to leave %{name}?" links: "Links" login_to_join: "Please login to join the community." members: "Members" @@ -550,6 +557,9 @@ en: create: failure: The community member could not be created. success: Member was successfully created. + destroy: + failure: Member was not removed. + success: Member was removed. edit: edit_member: Editing Member index: @@ -557,6 +567,7 @@ en: member: Member members: Members organizers: Organizers + remove: remove update: failure: Community Member could not be updated. success: Community Member was successfully updated. diff --git a/config/routes.rb b/config/routes.rb index 03c11d65ad9..4f1bd4e12a7 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -333,7 +333,7 @@ end post "/communities/:id/step_up" => "communities#step_up", :as => :step_up, :id => /\d+/ resources :community_links, :only => [:destroy, :edit, :update] - resources :community_members, :only => [:create, :edit, :new, :update] + resources :community_members, :only => [:create, :destroy, :edit, :new, :update] get "/community_members" => "community_members#create", :as => "login_to_join" # errors diff --git a/test/controllers/community_members_controller_test.rb b/test/controllers/community_members_controller_test.rb index a9ea2d66b08..8224561fb5f 100644 --- a/test/controllers/community_members_controller_test.rb +++ b/test/controllers/community_members_controller_test.rb @@ -131,4 +131,63 @@ def test_update_when_save_fails assert_response :success assert_template "community_members/edit" end + + def test_delete_as_a_different_user + # arrange + cm = create(:community_member) # N.b. not an organizer + session_for(create(:user)) + + # act + delete community_member_url(cm), :xhr => true + + # assert + follow_redirect! + assert_response :forbidden + end + + def test_delete_when_destroy_works + # arrange + cm1 = create(:community_member, :organizer) # original + session_for(cm1.user) + cm2 = create(:community_member, :community => cm1.community) + + # act + delete community_member_url(cm2), :xhr => true + + # assert + cm1.reload + assert_not cm1.community.member?(cm2.user) + end + + def test_delete_when_destroy_fails + # arrange + cm = create(:community_member, :organizer) + session_for(cm.user) + # Customize this instance so delete returns false. + def cm.destroy + false + end + + controller_mock = CommunityMembersController.new + def controller_mock.set_community_member + @community_member = CommunityMember.new + end + + def controller_mock.render(_partial, _msg) + # Can't do assert_equal here. + # assert_equal :edit, partial + end + + # act + CommunityMembersController.stub :new, controller_mock do + CommunityMember.stub :new, cm do + assert_difference "CommunityMember.count", 0 do + delete community_member_url(cm), :xhr => true + end + end + end + + # assert + assert_match(/#{I18n.t("community_members.destroy.failure")}/, flash[:error]) + end end