diff --git a/Gemfile.lock b/Gemfile.lock index cf5998618..21c0d2371 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -402,7 +402,8 @@ GEM http-cookie (>= 1.0.2, < 2.0) mime-types (>= 1.16, < 4.0) netrc (~> 0.8) - rexml (3.2.5) + rexml (3.2.8) + strscan (>= 3.0.9) rspec (3.12.0) rspec-core (~> 3.12.0) rspec-expectations (~> 3.12.0) @@ -487,6 +488,7 @@ GEM stringex (2.8.6) strong_migrations (1.4.4) activerecord (>= 5.2) + strscan (3.1.0) ten_years_rails (0.2.0) actionview activesupport diff --git a/app/controllers/api/v1/memberships_controller.rb b/app/controllers/api/v1/memberships_controller.rb index c46ac7af4..fc4a6a325 100644 --- a/app/controllers/api/v1/memberships_controller.rb +++ b/app/controllers/api/v1/memberships_controller.rb @@ -6,7 +6,7 @@ class Api::V1::MembershipsController < Api::ApiController resource_actions :index, :show, :create, :update, :deactivate schema_type :strong_params - allowed_params :update, :state + allowed_params :update, :state, roles: [] def create resources = resource_class.transaction(requires_new: true) do diff --git a/app/models/user.rb b/app/models/user.rb index 384064897..b620d8fcb 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -50,7 +50,7 @@ class User < ApplicationRecord validates :login, presence: true, format: { with: USER_LOGIN_REGEX } validates_uniqueness_of :login, case_sensitive: false validates :display_name, presence: true - validates :unsubscribe_token, presence: true, uniqueness: true + validates :unsubscribe_token, presence: true, uniqueness: true, on: :create validates_inclusion_of :valid_email, in: [true, false], message: "must be true or false" validates_with IdentityGroupNameValidator 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/lib/project_copier.rb b/lib/project_copier.rb index 3edd513ea..e6f458917 100644 --- a/lib/project_copier.rb +++ b/lib/project_copier.rb @@ -11,7 +11,7 @@ class ProjectCopier launched_row_order beta_row_order ].freeze - + INCLUDE_ASSOCIATIONS = [ :tutorials, :pages, @@ -61,7 +61,7 @@ def copy_project copied_project = project_to_copy.deep_clone include: INCLUDE_ASSOCIATIONS, except: EXCLUDE_ATTRIBUTES copied_project.owner = user - copied_project.display_name += ' (copy)' if user == project_to_copy.owner + copied_project.display_name += " (copy: #{Time.now.utc.strftime('%Y-%m-%d %H:%M:%S')})" copied_project.assign_attributes(launch_approved: false, live: false) # reset the project's configuration but record the source project id diff --git a/lib/tasks/user.rake b/lib/tasks/user.rake index ab6be1f67..5622ea90b 100644 --- a/lib/tasks/user.rake +++ b/lib/tasks/user.rake @@ -11,6 +11,14 @@ namespace :user do end end + desc 'Scrub unsubscribe_token of deactivated users' + task scrub_inactive_user_unsubscribe_token: :environment do + User.where(activated_state: 'inactive').select(:id).find_in_batches do |users| + ids = users.map(&:id) + User.where(id: ids).update_all(unsubscribe_token: nil) + end + end + desc 'Backfill UX testing emails comms field in batches' task backfill_ux_testing_email_field: :environment do User.select(:id).find_in_batches do |users| diff --git a/lib/user_info_scrubber.rb b/lib/user_info_scrubber.rb index 1a0fe2d09..a42b23692 100644 --- a/lib/user_info_scrubber.rb +++ b/lib/user_info_scrubber.rb @@ -29,6 +29,7 @@ def self.scrub_details(user) user.valid_email = false user.private_profile = true user.api_key = nil + user.unsubscribe_token = nil user.tsv = nil user.save! user diff --git a/spec/controllers/api/v1/memberships_controller_spec.rb b/spec/controllers/api/v1/memberships_controller_spec.rb index a39289ff6..a6ebe9d9f 100644 --- a/spec/controllers/api/v1/memberships_controller_spec.rb +++ b/spec/controllers/api/v1/memberships_controller_spec.rb @@ -35,6 +35,23 @@ end it_behaves_like "is updatable" + + context 'when updating roles as group_admin' do + let(:other_user) { create(:user) } + let(:other_user_group) { create(:user_group, admin: authorized_user, private: true) } + let(:other_user_membership) { create(:membership, user: other_user, user_group: other_user_group, roles: ['group_member']) } + + it 'allows update of membership roles' do + default_request user_id: authorized_user.id, scopes: scopes + params = { + memberships: { roles: ['group_admin'] }, + id: other_user_membership.id + } + + put :update, params: params + expect(other_user_membership.reload.roles).to eq(['group_admin']) + end + end end describe "#create" do diff --git a/spec/lib/project_copier_spec.rb b/spec/lib/project_copier_spec.rb index 46cf657f8..407efa56b 100644 --- a/spec/lib/project_copier_spec.rb +++ b/spec/lib/project_copier_spec.rb @@ -16,13 +16,9 @@ expect(copied_project.owner).to eq(copyist) end - it 'renames a project when the owner is copying their own project' do - new_copy = described_class.new(project.id, project.owner.id).copy - expect(new_copy.display_name).to include('(copy)') - end - - it 'has matching attributes' do - expect(copied_project.display_name).to eq(project.display_name) + it 'appends copy and current timestamp to display_name' do + allow(Time).to receive(:now).and_return(Time.utc(2024, 5, 17, 12, 0, 0)) + expect(copied_project.display_name).to eq("#{project.display_name} (copy: 2024-05-17 12:00:00)") end it 'resets the live attribute' do diff --git a/spec/lib/user_info_scrubber_spec.rb b/spec/lib/user_info_scrubber_spec.rb index fc8781d1d..64a65cf59 100644 --- a/spec/lib/user_info_scrubber_spec.rb +++ b/spec/lib/user_info_scrubber_spec.rb @@ -33,6 +33,10 @@ def scrub_user_details(user)(user) expect { described_class.scrub_personal_info!(user) }.to change(user, :credited_name).to(nil) end + it 'scrubs the unsubscribe_token' do + expect { described_class.scrub_personal_info!(user) }.to change(user, :unsubscribe_token).to(nil) + end + it 'scrubs the encrypted_password' do expect { described_class.scrub_personal_info!(user) }.to change(user, :encrypted_password) end diff --git a/spec/policies/membership_policy_spec.rb b/spec/policies/membership_policy_spec.rb index 292d1dff6..6ef0ac79b 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) } @@ -31,44 +36,58 @@ 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 include(membership1) - expect(scope.resolve(:show)).to include(membership1) - 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 include(membership1, membership2, membership3, membership4) - expect(scope.resolve(:show)).to include(membership1, membership2, membership3, membership4) + 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)).not_to include(membership3, membership4) - expect(scope.resolve(:destroy)).not_to include(membership3, membership4) + 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 + ) end end @@ -79,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 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