From 026c9aa3b29aa6168bff2f4acaeda2151576f2b9 Mon Sep 17 00:00:00 2001 From: yuenmichelle1 Date: Fri, 17 May 2024 11:29:44 -0500 Subject: [PATCH 1/6] ensure unique memberships when querying for ones own public memberships --- app/policies/membership_policy.rb | 2 +- spec/policies/membership_policy_spec.rb | 25 +++++++++++++++---------- 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/app/policies/membership_policy.rb b/app/policies/membership_policy.rb index e22c5b3a9..6e46d8842 100644 --- a/app/policies/membership_policy.rb +++ b/app/policies/membership_policy.rb @@ -20,7 +20,7 @@ def resolve(action) when :show, :index public_memberships = scope.where(identity: false, state: Membership.states[:active]) .joins(:user_group).merge(UserGroup.where(private: false)) - Membership.union_all(query, public_memberships) + Membership.union_all(query, public_memberships).distinct else query end diff --git a/spec/policies/membership_policy_spec.rb b/spec/policies/membership_policy_spec.rb index 292d1dff6..44fea8a0f 100644 --- a/spec/policies/membership_policy_spec.rb +++ b/spec/policies/membership_policy_spec.rb @@ -18,6 +18,11 @@ MembershipPolicy::Scope.new(api_user, Membership) end + let(:group_admin_public_membership) { Membership.where(user_id: group_admin.id, user_group_id: public_user_group.id, state: 'active')[0] } + let(:group_admin_private_membership) { + Membership.where(user_id: group_admin.id, user_group_id: private_user_group.id, state: 'active')[0] + } + context 'an anonymous user' do let(:api_user) { ApiUser.new(anonymous_user) } @@ -38,8 +43,8 @@ membership2 = create :membership, user: other_user, user_group: private_user_group membership3 = create :membership, user_group: public_user_group, state: :inactive - expect(scope.resolve(:index)).to include(membership1) - expect(scope.resolve(:show)).to include(membership1) + expect(scope.resolve(:index)).to contain_exactly(membership1, group_admin_public_membership) + expect(scope.resolve(:show)).to contain_exactly(membership1, group_admin_public_membership) expect(scope.resolve(:index)).not_to include(membership2) expect(scope.resolve(:show)).not_to include(membership2) expect(scope.resolve(:index)).not_to include(membership3) @@ -53,8 +58,8 @@ membership3 = create :membership, user: other_user, user_group: public_user_group membership4 = create :membership, user: other_user, user_group: private_user_group - expect(scope.resolve(:index)).to include(membership1, membership2, membership3, membership4) - expect(scope.resolve(:show)).to include(membership1, membership2, membership3, membership4) + expect(scope.resolve(:index)).to contain_exactly(membership1, membership2, membership3, membership4, group_admin_public_membership, group_admin_private_membership) + expect(scope.resolve(:show)).to contain_exactly(membership1, membership2, membership3, membership4, group_admin_private_membership, group_admin_public_membership) end it "cannot modify other user's memberships" do @@ -67,8 +72,8 @@ expect(scope.resolve(:update)).to include(membership1, membership2) expect(scope.resolve(:destroy)).to include(membership1, membership2) - expect(scope.resolve(:update)).not_to include(membership3, membership4) - expect(scope.resolve(:destroy)).not_to include(membership3, membership4) + expect(scope.resolve(:update)).not_to include(membership3, membership4, group_admin_private_membership, group_admin_public_membership) + expect(scope.resolve(:destroy)).not_to include(membership3, membership4, group_admin_private_membership, group_admin_public_membership) end end @@ -79,10 +84,10 @@ membership1 = create :membership, user: logged_in_user, user_group: public_user_group membership2 = create :membership, user: logged_in_user, user_group: private_user_group - expect(scope.resolve(:index)).to include(membership1, membership2) - expect(scope.resolve(:show)).to include(membership1, membership2) - expect(scope.resolve(:update)).to include(membership1, membership2) - expect(scope.resolve(:destroy)).to include(membership1, membership2) + expect(scope.resolve(:index)).to contain_exactly(membership1, membership2, group_admin_private_membership, group_admin_public_membership) + expect(scope.resolve(:show)).to contain_exactly(membership1, membership2, group_admin_private_membership, group_admin_public_membership) + expect(scope.resolve(:update)).to include(membership1, membership2, group_admin_private_membership, group_admin_public_membership) + expect(scope.resolve(:destroy)).to include(membership1, membership2, group_admin_private_membership, group_admin_public_membership) end end end From d371c1183fad22b2150a7939e117c85278dfe375 Mon Sep 17 00:00:00 2001 From: yuenmichelle1 Date: Fri, 17 May 2024 11:55:27 -0500 Subject: [PATCH 2/6] update membership policy spec with clearer variable names and spec clean up --- spec/policies/membership_policy_spec.rb | 48 ++++++++++--------------- 1 file changed, 19 insertions(+), 29 deletions(-) diff --git a/spec/policies/membership_policy_spec.rb b/spec/policies/membership_policy_spec.rb index 44fea8a0f..41fc2b55e 100644 --- a/spec/policies/membership_policy_spec.rb +++ b/spec/policies/membership_policy_spec.rb @@ -36,44 +36,34 @@ context 'a normal user', :aggregate_failures do let(:api_user) { ApiUser.new(logged_in_user) } + let(:other_user) { create(:user) } + let(:other_user_public_membership) { create(:membership, user: other_user, user_group: public_user_group) } + let(:other_user_private_membership) { create(:membership, user: other_user, user_group: private_user_group) } + let(:logged_in_user_public_membership) { create(:membership, user: logged_in_user, user_group: public_user_group) } + let(:logged_in_user_private_membership) { create(:membership, user: logged_in_user, user_group: private_user_group) } it 'can see other people in public groups' do - other_user = create :user - membership1 = create :membership, user: other_user, user_group: public_user_group - membership2 = create :membership, user: other_user, user_group: private_user_group - membership3 = create :membership, user_group: public_user_group, state: :inactive - - expect(scope.resolve(:index)).to contain_exactly(membership1, group_admin_public_membership) - expect(scope.resolve(:show)).to contain_exactly(membership1, group_admin_public_membership) - expect(scope.resolve(:index)).not_to include(membership2) - expect(scope.resolve(:show)).not_to include(membership2) - expect(scope.resolve(:index)).not_to include(membership3) - expect(scope.resolve(:show)).not_to include(membership3) + inactive_membership = create :membership, user_group: public_user_group, state: :inactive + + expect(scope.resolve(:index)).to contain_exactly(other_user_public_membership, group_admin_public_membership) + expect(scope.resolve(:show)).to contain_exactly(other_user_public_membership, group_admin_public_membership) + expect(scope.resolve(:index)).not_to include(other_user_private_membership) + expect(scope.resolve(:show)).not_to include(other_user_private_membership) + expect(scope.resolve(:index)).not_to include(inactive_membership) + expect(scope.resolve(:show)).not_to include(inactive_membership) end it 'can see who are members of private groups they are in' do - other_user = create :user - membership1 = create :membership, user: logged_in_user, user_group: public_user_group - membership2 = create :membership, user: logged_in_user, user_group: private_user_group - membership3 = create :membership, user: other_user, user_group: public_user_group - membership4 = create :membership, user: other_user, user_group: private_user_group - - expect(scope.resolve(:index)).to contain_exactly(membership1, membership2, membership3, membership4, group_admin_public_membership, group_admin_private_membership) - expect(scope.resolve(:show)).to contain_exactly(membership1, membership2, membership3, membership4, group_admin_private_membership, group_admin_public_membership) + expect(scope.resolve(:index)).to contain_exactly(logged_in_user_public_membership, logged_in_user_private_membership, other_user_public_membership, other_user_private_membership, group_admin_public_membership, group_admin_private_membership) + expect(scope.resolve(:show)).to contain_exactly(logged_in_user_public_membership, logged_in_user_private_membership, other_user_public_membership, other_user_private_membership, group_admin_public_membership, group_admin_private_membership) end it "cannot modify other user's memberships" do - other_user = create :user - membership1 = create :membership, user: logged_in_user, user_group: public_user_group - membership2 = create :membership, user: logged_in_user, user_group: private_user_group - membership3 = create :membership, user: other_user, user_group: public_user_group - membership4 = create :membership, user: other_user, user_group: private_user_group - - expect(scope.resolve(:update)).to include(membership1, membership2) - expect(scope.resolve(:destroy)).to include(membership1, membership2) + expect(scope.resolve(:update)).to include(logged_in_user_public_membership, logged_in_user_private_membership) + expect(scope.resolve(:destroy)).to include(logged_in_user_public_membership, logged_in_user_private_membership) - expect(scope.resolve(:update)).not_to include(membership3, membership4, group_admin_private_membership, group_admin_public_membership) - expect(scope.resolve(:destroy)).not_to include(membership3, membership4, group_admin_private_membership, group_admin_public_membership) + expect(scope.resolve(:update)).not_to include(other_user_private_membership, other_user_public_membership, group_admin_private_membership, group_admin_public_membership) + expect(scope.resolve(:destroy)).not_to include(other_user_private_membership, other_user_public_membership, group_admin_private_membership, group_admin_public_membership) end end From 9007d255bce8c3bdd0a3668be27e9adba4fdf66f Mon Sep 17 00:00:00 2001 From: yuenmichelle1 Date: Fri, 17 May 2024 11:57:12 -0500 Subject: [PATCH 3/6] hound remove extra spacing --- spec/policies/membership_policy_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/policies/membership_policy_spec.rb b/spec/policies/membership_policy_spec.rb index 41fc2b55e..f715e2692 100644 --- a/spec/policies/membership_policy_spec.rb +++ b/spec/policies/membership_policy_spec.rb @@ -36,7 +36,7 @@ context 'a normal user', :aggregate_failures do let(:api_user) { ApiUser.new(logged_in_user) } - let(:other_user) { create(:user) } + let(:other_user) { create(:user) } let(:other_user_public_membership) { create(:membership, user: other_user, user_group: public_user_group) } let(:other_user_private_membership) { create(:membership, user: other_user, user_group: private_user_group) } let(:logged_in_user_public_membership) { create(:membership, user: logged_in_user, user_group: public_user_group) } From e8fbcc51d569b9f2152dfd81500ceb7a83813897 Mon Sep 17 00:00:00 2001 From: yuenmichelle1 Date: Mon, 20 May 2024 09:56:09 -0500 Subject: [PATCH 4/6] Update spec/policies/membership_policy_spec.rb Co-authored-by: Zach Wolfenbarger --- spec/policies/membership_policy_spec.rb | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/spec/policies/membership_policy_spec.rb b/spec/policies/membership_policy_spec.rb index f715e2692..da1e7c60a 100644 --- a/spec/policies/membership_policy_spec.rb +++ b/spec/policies/membership_policy_spec.rb @@ -54,7 +54,14 @@ end it 'can see who are members of private groups they are in' do - expect(scope.resolve(:index)).to contain_exactly(logged_in_user_public_membership, logged_in_user_private_membership, other_user_public_membership, other_user_private_membership, group_admin_public_membership, group_admin_private_membership) + expect(scope.resolve(:index)).to contain_exactly( + logged_in_user_public_membership, + logged_in_user_private_membership, + other_user_public_membership, + other_user_private_membership, + group_admin_public_membership, + group_admin_private_membership + ) expect(scope.resolve(:show)).to contain_exactly(logged_in_user_public_membership, logged_in_user_private_membership, other_user_public_membership, other_user_private_membership, group_admin_public_membership, group_admin_private_membership) end From 00f3d19e566cea5a70d259c0e9e0b6b11f6d2f12 Mon Sep 17 00:00:00 2001 From: yuenmichelle1 Date: Mon, 20 May 2024 09:56:15 -0500 Subject: [PATCH 5/6] Update spec/policies/membership_policy_spec.rb Co-authored-by: Zach Wolfenbarger --- spec/policies/membership_policy_spec.rb | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/spec/policies/membership_policy_spec.rb b/spec/policies/membership_policy_spec.rb index da1e7c60a..daef70ba0 100644 --- a/spec/policies/membership_policy_spec.rb +++ b/spec/policies/membership_policy_spec.rb @@ -62,7 +62,14 @@ group_admin_public_membership, group_admin_private_membership ) - expect(scope.resolve(:show)).to contain_exactly(logged_in_user_public_membership, logged_in_user_private_membership, other_user_public_membership, other_user_private_membership, group_admin_public_membership, group_admin_private_membership) + expect(scope.resolve(:show)).to contain_exactly( + logged_in_user_public_membership, + logged_in_user_private_membership, + other_user_public_membership, + other_user_private_membership, + group_admin_public_membership, + group_admin_private_membership + ) end it "cannot modify other user's memberships" do From 69d3edd580dfe4cae46259833531c8784c652d5a Mon Sep 17 00:00:00 2001 From: yuenmichelle1 Date: Mon, 20 May 2024 09:59:37 -0500 Subject: [PATCH 6/6] update membership policy spec spacing format when expecting a long list of memberships --- spec/policies/membership_policy_spec.rb | 42 +++++++++++++++++++++---- 1 file changed, 36 insertions(+), 6 deletions(-) diff --git a/spec/policies/membership_policy_spec.rb b/spec/policies/membership_policy_spec.rb index daef70ba0..6ef0ac79b 100644 --- a/spec/policies/membership_policy_spec.rb +++ b/spec/policies/membership_policy_spec.rb @@ -76,8 +76,18 @@ expect(scope.resolve(:update)).to include(logged_in_user_public_membership, logged_in_user_private_membership) expect(scope.resolve(:destroy)).to include(logged_in_user_public_membership, logged_in_user_private_membership) - expect(scope.resolve(:update)).not_to include(other_user_private_membership, other_user_public_membership, group_admin_private_membership, group_admin_public_membership) - expect(scope.resolve(:destroy)).not_to include(other_user_private_membership, other_user_public_membership, group_admin_private_membership, group_admin_public_membership) + expect(scope.resolve(:update)).not_to include( + other_user_private_membership, + other_user_public_membership, + group_admin_private_membership, + group_admin_public_membership + ) + expect(scope.resolve(:destroy)).not_to include( + other_user_private_membership, + other_user_public_membership, + group_admin_private_membership, + group_admin_public_membership + ) end end @@ -88,10 +98,30 @@ membership1 = create :membership, user: logged_in_user, user_group: public_user_group membership2 = create :membership, user: logged_in_user, user_group: private_user_group - expect(scope.resolve(:index)).to contain_exactly(membership1, membership2, group_admin_private_membership, group_admin_public_membership) - expect(scope.resolve(:show)).to contain_exactly(membership1, membership2, group_admin_private_membership, group_admin_public_membership) - expect(scope.resolve(:update)).to include(membership1, membership2, group_admin_private_membership, group_admin_public_membership) - expect(scope.resolve(:destroy)).to include(membership1, membership2, group_admin_private_membership, group_admin_public_membership) + expect(scope.resolve(:index)).to contain_exactly( + membership1, + membership2, + group_admin_private_membership, + group_admin_public_membership + ) + expect(scope.resolve(:show)).to contain_exactly( + membership1, + membership2, + group_admin_private_membership, + group_admin_public_membership + ) + expect(scope.resolve(:update)).to include( + membership1, + membership2, + group_admin_private_membership, + group_admin_public_membership + ) + expect(scope.resolve(:destroy)).to include( + membership1, + membership2, + group_admin_private_membership, + group_admin_public_membership + ) end end end