diff --git a/Gemfile.lock b/Gemfile.lock index 028b49297..cf5998618 100644 --- a/Gemfile.lock +++ b/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 diff --git a/app/controllers/api/v1/project_preferences_controller.rb b/app/controllers/api/v1/project_preferences_controller.rb index 476548c96..b744c3bf5 100644 --- a/app/controllers/api/v1/project_preferences_controller.rb +++ b/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 diff --git a/app/controllers/api/v1/users_controller.rb b/app/controllers/api/v1/users_controller.rb index 672239451..5ea2507f1 100644 --- a/app/controllers/api/v1/users_controller.rb +++ b/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 diff --git a/lib/project_copier.rb b/lib/project_copier.rb index a4bbc5ed0..3edd513ea 100644 --- a/lib/project_copier.rb +++ b/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, diff --git a/spec/controllers/api/v1/project_preferences_controller_spec.rb b/spec/controllers/api/v1/project_preferences_controller_spec.rb index 84f45d185..d43f33f8e 100644 --- a/spec/controllers/api/v1/project_preferences_controller_spec.rb +++ b/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 diff --git a/spec/controllers/api/v1/users_controller_spec.rb b/spec/controllers/api/v1/users_controller_spec.rb index 6e042757c..d28cb8585 100644 --- a/spec/controllers/api/v1/users_controller_spec.rb +++ b/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 diff --git a/spec/lib/project_copier_spec.rb b/spec/lib/project_copier_spec.rb index 4a01ceaf2..46cf657f8 100644 --- a/spec/lib/project_copier_spec.rb +++ b/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