diff --git a/app/services/membership_service.rb b/app/services/membership_service.rb index 43f4abe42db..f2c35ecaf16 100644 --- a/app/services/membership_service.rb +++ b/app/services/membership_service.rb @@ -10,48 +10,52 @@ def self.redeem(membership:, actor:, notify: true) # and we dont want any surprises if they already have some memberships. # they may be accepting memberships send to a different email (unverified_user) - group_ids = membership.group.parent_or_self.id_and_subgroup_ids + invited_group_id = membership.group_id + existing_group_ids = Membership.where(user_id: actor.id).pluck(:group_id) + existing_accepted_group_ids = Membership.active.accepted.where(user_id: actor.id).pluck(:group_id) + invited_group_ids = Membership.pending.where(user_id: membership.user_id, group_id: membership.group.parent_or_self.id_and_subgroup_ids).pluck(:group_id) + + # unrevoke any memberships the actor was just invited to + Membership.revoked + .where(user_id: actor.id, group_id: invited_group_ids) + .update(revoked_at: nil, revoker_id: nil, inviter_id: membership.inviter_id, accepted_at: DateTime.now) # ensure actor has accepted any existing pending memberships to this group Membership.pending - .where(user_id: actor.id, group_id: group_ids) - .update_all(accepted_at: DateTime.now) - - # cant accept pending memberships to groups I already belong to - existing_group_ids = Membership.pending.where(user_id: membership.user_id, - group_id: actor.memberships.accepted.pluck(:group_id)).pluck(:group_id) + .where(user_id: actor.id, group_id: invited_group_ids) + .update(accepted_at: DateTime.now) - Membership.pending.where( - user_id: membership.user_id, - group_id: (group_ids - existing_group_ids)) - .update_all( - user_id: actor.id, - accepted_at: DateTime.now) + Membership.pending + .where(user_id: membership.user_id, group_id: (invited_group_ids - existing_group_ids)) + .update(user_id: actor.id, accepted_at: DateTime.now) - member_group_ids = actor.group_ids & membership.group.parent_or_self.id_and_subgroup_ids + if (membership.user_id != actor.id) + Membership.where(user_id: membership.user_id, group_id: invited_group_ids).destroy_all + end - group_ids.each do |group_id| + invited_group_ids.each do |group_id| GenericWorker.perform_async('PollService', 'group_members_added', group_id) end # remove any existing guest access in these groups DiscussionReader.joins(:discussion) - .where(user_id: actor.id, 'discussions.group_id': member_group_ids, guest: true) + .where(user_id: actor.id, 'discussions.group_id': invited_group_ids, guest: true) .update_all(guest: false, revoked_at: nil, revoker_id: nil) Stance.joins(:poll) - .where(participant_id: actor.id, 'polls.group_id': member_group_ids) + .where(participant_id: actor.id, 'polls.group_id': invited_group_ids) .update_all(guest: false) # unrevoke any votes on active polls Stance.joins(:poll) .where(participant_id: actor.id) - .where('polls.group_id': member_group_ids) + .where('polls.group_id': invited_group_ids) .where('stances.revoked_at is not null') .where('polls.closed_at is null') .update_all(revoked_at: nil, revoker_id: nil) - membership.reload + return if existing_accepted_group_ids.include?(invited_group_id) + membership = Membership.find_by!(group_id: invited_group_id, user_id: actor.id) Events::InvitationAccepted.publish!(membership) if notify && membership.accepted_at end diff --git a/spec/controllers/memberships_controller_spec.rb b/spec/controllers/memberships_controller_spec.rb index 028b0660394..e25b8da620f 100644 --- a/spec/controllers/memberships_controller_spec.rb +++ b/spec/controllers/memberships_controller_spec.rb @@ -44,9 +44,7 @@ expect(membership.accepted_at).to eq nil expect {get :consume}.to_not change { Event.count } expect(response.status).to eq 200 - membership.reload - expect(membership.accepted_at).to eq nil - expect(membership.user_id).to_not eq user.id + expect(Membership.where(id: membership.id).exists?).to be false end it "does not redeem accepted membership" do diff --git a/spec/services/membership_service_spec.rb b/spec/services/membership_service_spec.rb index a3f9cf1b40d..0ba94a9273f 100644 --- a/spec/services/membership_service_spec.rb +++ b/spec/services/membership_service_spec.rb @@ -40,10 +40,49 @@ let!(:another_subgroup) { create :group, parent: group } let!(:discussion) { create :discussion, group: group, private: false } let!(:poll) { create :poll, group: group } + let(:first_inviter) { create :user } + let(:second_inviter) { create :user } + let(:yesterday) { 1.day.ago } + let(:today) { DateTime.now } + + before do + group.add_admin! first_inviter + group.add_admin! second_inviter + end it 'sets accepted_at' do MembershipService.redeem(membership: membership, actor: user) - membership.accepted_at.should be_present + membership.reload.accepted_at.should be_present + end + + it "handles simple case" do + new_membership = Membership.create!(user_id: user.id, group_id: group.id, inviter_id: first_inviter.id) + MembershipService.redeem(membership: new_membership, actor: user) + expect(new_membership.reload.user_id).to eq user.id + expect(new_membership.reload.accepted_at).to be_present + expect(new_membership.reload.inviter_id).to eq first_inviter.id + expect(new_membership.reload.revoked_at).to_not be_present + end + + it "handles existing memberships" do + existing_membership = Membership.create!(user_id: user.id, group_id: group.id, accepted_at: yesterday, inviter_id: first_inviter.id) + new_membership = Membership.create!(user_id: unverified_user.id, group_id: group.id, inviter_id: second_inviter.id) + MembershipService.redeem(membership: new_membership, actor: user) + expect(existing_membership.reload.user_id).to eq user.id + expect(existing_membership.reload.accepted_at).to_not be yesterday + expect(existing_membership.reload.accepted_at).to be_present + expect(existing_membership.reload.inviter_id).to eq first_inviter.id + expect(existing_membership.reload.revoked_at).to_not be_present + end + + it "handles revoked memberships" do + existing_membership = Membership.create!(user_id: user.id, group_id: group.id, accepted_at: DateTime.now, revoked_at: DateTime.now, inviter_id: first_inviter.id, revoker_id: first_inviter.id) + new_membership = Membership.create!(user_id: unverified_user.id, group_id: group.id, inviter_id: second_inviter.id) + MembershipService.redeem(membership: new_membership, actor: user) + expect(existing_membership.reload.user_id).to eq user.id + expect(existing_membership.reload.inviter_id).to eq second_inviter.id + expect(existing_membership.reload.accepted_at).to be_present + expect(existing_membership.reload.revoked_at).to_not be_present end it 'unrevokes discussion readers and stances' do