Commit d27fda55 authored by dgt's avatar dgt

Merge branch '282-fix-500-on-double-join' into 'master'

Resolve "500 when adding user twice to group"

Closes #282

See merge request !264
parents 823dd466 5e50d4f3
......@@ -37,10 +37,15 @@ module Common::Requests
# uses model permissions.
skip_authorization
if mark
@req.mark!(mark, current_user)
success I18n.t(@req.name), success_message
begin
@req.mark!(mark, current_user)
success I18n.t(@req.name), success_message
rescue Request::PointlessAction => e
status = 409
notice I18n.t(@req.name), e.message
end
end
render template: 'common/requests/update'
render template: 'common/requests/update', status: status
end
#
......
......@@ -154,7 +154,10 @@ module Group::Users
#
def remove_user!(user)
membership = memberships.find_by_user_id(user.id)
raise ErrorMessage.new('no such membership') unless membership
unless membership
raise ErrorMessage,
I18n.t(:no_such_membership_error, group: self.name, user: user.name)
end
# removing all participations (makes the stars disappear - not sure
# if we want this)
......@@ -177,7 +180,6 @@ module Group::Users
committees.each do |committe|
committe.remove_user!(user) unless committe.users.find_by_id(user.id).blank?
end
end
def open_membership?
......
......@@ -10,6 +10,8 @@ class MembershipRequest < Request
def after_approval
group.add_user! user
rescue AssociationError => e
raise PointlessAction, e.message
end
def event
......
......@@ -30,6 +30,12 @@
class Request < ActiveRecord::Base
include AASM
# Raised when approving a Request to do something
# that does not have any consequences.
# (Such as removing a user from a group who is not a member)
class PointlessAction < CrabgrassException
end
##
## ASSOCIATIONS
##
......
......@@ -36,6 +36,8 @@ class RequestToJoinYou < MembershipRequest
def after_approval
group.add_user! created_by
rescue AssociationError => e
raise PointlessAction, e.message
end
def description
......
......@@ -50,6 +50,8 @@ class RequestToRemoveUser < Request
def after_approval
group.remove_user!(user)
rescue ErrorMessage => e
raise PointlessAction, e.message
end
def description
......
......@@ -56,6 +56,7 @@ en:
group_membership_count: "%{count} Members"
membership_destroy_confirm_message: "Are you sure you want to remove %{entity} from %{group}?"
membership_exists_error: "Membership already exists for %{member}"
no_such_membership_error: "%{user} is currently not part of %{group}"
membership_leave_message: "You have been removed from %{group}"
membership_notification: "Membership notification"
membership_requests: "Membership Requests"
......
......@@ -15,33 +15,65 @@ class Me::RequestsControllerTest < ActionController::TestCase
end
def test_approve_friend_request
@user = FactoryBot.create(:user)
user = FactoryBot.create(:user)
requesting = FactoryBot.create(:user)
request = RequestToFriend.create created_by: requesting,
recipient: @user
login_as @user
recipient: user
login_as user
xhr :post, :update, id: request.id, mark: 'approve'
assert_response :success
end
def test_approve_group_request
@user = FactoryBot.create(:user)
@group = FactoryBot.create(:group)
@group.add_user! @user
login_as @user
user = FactoryBot.create(:user)
group = FactoryBot.create(:group)
group.add_user! user
login_as user
requesting = FactoryBot.create(:user)
request = RequestToJoinYou.create created_by: requesting,
recipient: @group
recipient: group
xhr :post, :update, id: request.id, mark: 'approve'
assert_response :success
end
def test_dup_group_join_request
user = FactoryBot.create(:user)
group = FactoryBot.create(:group)
group.add_user! user
login_as user
requesting = FactoryBot.create(:user)
request = RequestToJoinYou.create created_by: requesting,
recipient: group
group.add_user! requesting
xhr :post, :update, id: request.id, mark: 'approve'
assert_response :conflict
end
def test_dup_remove_from_group_request
user = FactoryBot.create(:user)
requesting = FactoryBot.create(:user)
remove_me = FactoryBot.create(:user)
group = FactoryBot.create(:group)
group.add_user! user
group.add_user! requesting
group.add_user! remove_me
request = RequestToRemoveUser.create created_by: requesting,
recipient: group,
requestable: remove_me
login_as user
group.remove_user! remove_me
xhr :post, :update, id: request.id, mark: 'approve'
assert_response :conflict
end
def test_destroy_group_request
@user = FactoryBot.create(:user)
@group = FactoryBot.create(:group)
@group.add_user! @user
login_as @user
user = FactoryBot.create(:user)
group = FactoryBot.create(:group)
group.add_user! user
login_as user
requesting = FactoryBot.create(:user)
request = RequestToJoinYou.create created_by: requesting,
recipient: @group
recipient: group
assert_difference 'RequestToJoinYou.count', -1 do
xhr :delete, :destroy, id: request.id
end
......@@ -49,8 +81,8 @@ class Me::RequestsControllerTest < ActionController::TestCase
end
def test_other_requests_hidden
@user = FactoryBot.create(:user)
login_as @user
user = FactoryBot.create(:user)
login_as user
assert_not_found do
get :show, id: Request.last
end
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment