Merge branch 'master' into dependabot/bundler/docs/rexml-3.2.8

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

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

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

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

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

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|

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

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

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

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

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