Merge branch 'master' into user_group_search_mimic_user_search

Gemfile.lock

@@ -348,7 +348,7 @@ GEM
       activesupport (>= 3.0.0)
     raabro (1.4.0)
     racc (1.7.1)
-    rack (2.2.8)
+    rack (2.2.8.1)
     rack-cors (1.1.1)
       rack (>= 2.0.0)
     rack-protection (3.0.5)
@@ -510,12 +510,10 @@ GEM
       addressable (>= 2.8.0)
       crack (>= 0.3.2)
       hashdiff (>= 0.4.0, < 2.0.0)
-    webrick (1.7.0)
     websocket-driver (0.7.5)
       websocket-extensions (>= 0.1.0)
     websocket-extensions (0.1.5)
-    yard (0.9.27)
-      webrick (~> 1.7.0)
+    yard (0.9.36)
     zeitwerk (2.6.10)
     zoo_stream (1.0.1)
       aws-sdk

app/controllers/api/v1/project_preferences_controller.rb

@@ -6,16 +6,21 @@ class Api::V1::ProjectPreferencesController < Api::ApiController
   resource_actions :create, :update, :show, :index, :update_settings
   extra_schema_actions :update_settings
   schema_type :json_schema
-  before_action :find_upp, only: %i[update_settings read_settings]
+  before_action :find_upp, only: %i[update_settings]
 
   def read_settings
     skip_policy_scope
+    project = Project.find(params[:project_id])
+    raise Api::Unauthorized, 'You must be the project owner or a collaborator' unless user_allowed?(project)
+
+    upp_list = fetch_upp_list
+
     render(
       status: :ok,
       json_api: serializer.page(
         params,
-        @upp_list,
-        context
+        upp_list,
+        send_settings_context
       )
     )
   end
@@ -30,30 +35,38 @@ def update_settings
       status: :ok,
       json_api: serializer.resource(
         {},
-        @upp_list,
-        context
+        UserProjectPreference.where(id: @upp.id),
+        send_settings_context
       )
     )
   end
 
-  def fetch_upp_list
-    if action_name == 'read_settings'
-      @upp_list = UserProjectPreference.where(project_id: params[:project_id]).where.not(email_communication: nil)
-      @upp_list = @upp_list.where(user_id: params[:user_id]) if params[:user_id].present?
-    else
-      @upp_list = UserProjectPreference.where(user_id: params_for[:user_id], project_id: params_for[:project_id])
-    end
+  def find_upp
+    @upp = UserProjectPreference.find_by!(
+      user_id: params_for[:user_id],
+      project_id: params_for[:project_id]
+    )
+    raise Api::Unauthorized, 'You must be the project owner or a collaborator' unless user_allowed?(@upp.project)
   end
 
-  def find_upp
-    fetch_upp_list
-    @upp = @upp_list.first
-    raise ActiveRecord::RecordNotFound unless @upp.present?
+  def fetch_upp_list
+    upp_list = UserProjectPreference.where(project_id: params[:project_id]).where.not(email_communication: nil)
+    upp_list = upp_list.where(user_id: params[:user_id]) if params[:user_id].present?
+    upp_list
+  end
 
-    raise Api::Unauthorized, 'You must be the project owner or a collaborator' unless user_allowed?
+  def user_allowed?(project)
+    project.owners_and_collaborators.include?(api_user.user) || api_user.is_admin?
   end
 
-  def user_allowed?
-    @upp.project.owners_and_collaborators.include?(api_user.user) || api_user.is_admin?
+  def send_settings_context
+    {
+      include_settings?: true,
+      include_email_communication?: false,
+      include_legacy_count?: false,
+      include_preferences?: false,
+      include_activity_count?: false,
+      include_activity_count_by_workflow?: false
+    }
   end
 end

app/controllers/api/v1/users_controller.rb

@@ -52,6 +52,7 @@ def update
       super do |user|
         if user.email_changed?
           update_email_user_ids << user.id
+          user.update_attribute(:valid_email, true)
         end
       end
 

lib/project_copier.rb

@@ -1,7 +1,17 @@
 class ProjectCopier
   attr_reader :project_to_copy, :user
 
-  EXCLUDE_ATTRIBUTES = %i[classifications_count launched_row_order beta_row_order].freeze
+  EXCLUDE_ATTRIBUTES = %i[
+    classifications_count
+    classifiers_count
+    launch_date
+    completeness
+    activity
+    lock_version
+    launched_row_order
+    beta_row_order
+  ].freeze
+
   INCLUDE_ASSOCIATIONS = [
     :tutorials,
     :pages,

spec/controllers/api/v1/project_preferences_controller_spec.rb

@@ -221,6 +221,16 @@
         json_response = JSON.parse(response.body)
         expect(json_response['project_preferences'].count).to eq(2)
       end
+
+      # relevant attributes are id, href and settings from UPP
+      it 'returns the correct serialized attributes' do
+        json_response = JSON.parse(response.body)
+        first_response = json_response['project_preferences'].first
+
+        expect(first_response).to have_key 'settings'
+        expect(first_response).to_not have_key 'activity_count_by_workflow'
+        expect(first_response).to_not have_key 'email_communication'
+      end
     end
 
     describe 'invalid project' do
@@ -238,7 +248,7 @@
       it 'only fetches settings of owned project' do
         default_request user_id: unauthorised_user.id, scopes: scopes
         get :read_settings, params: { project_id: project.id, user_id: unauthorised_user.id, format: :json }
-        expect(response.status).to eq(404)
+        expect(response.status).to eq(403)
       end
 
       it 'only fetches settings of the specified user' do

spec/controllers/api/v1/users_controller_spec.rb

@@ -482,6 +482,13 @@ def update_request
         expect(UserInfoChangedMailerWorker).to receive(:perform_async).with(user.id, "email")
       end
 
+      it "sets valid_email parameter to true" do
+        # updating the valid email to false before the request as it is set at true by default
+        user.update(valid_email: false)
+        update_request
+        expect(user.reload.valid_email).to be_truthy
+      end
+
       describe "with an email that already exists" do
         let(:put_operations) { {users: {email: User.where.not(id: user.id).first.email}} }
         it "doesn't send an email to the new address if user is not valid" do

spec/lib/project_copier_spec.rb

@@ -41,6 +41,14 @@
       expect(copied_project.configuration['source_project_id']).to be(project.id)
     end
 
+    it 'does not copy over excluded attributes' do
+      project_with_excluded_keys = create(:full_project, classifications_count: 3, classifiers_count: 2, launch_date: Date.yesterday, completeness: 0.5, activity: 1, lock_version: 8)
+      other_copied_project = described_class.new(project_with_excluded_keys.id, copyist.id).copy
+      ProjectCopier::EXCLUDE_ATTRIBUTES.each do |attr|
+        expect(other_copied_project[attr]).not_to eq(project_with_excluded_keys[attr])
+      end
+    end
+
     it 'creates Talk roles for the new project and its owner' do
       allow(TalkAdminCreateWorker).to receive(:perform_async)
       copied_project