From 9a5631070f813122d665f025d66bbbd43887f39b Mon Sep 17 00:00:00 2001 From: Oluwatoyosi Oyegoke <34948675+Tooyosi@users.noreply.github.com> Date: Tue, 20 Feb 2024 20:15:06 +0000 Subject: [PATCH] implement project preference read settings endpoint (#4280) * implement project preference read settings endpoint * pr cleanups and add failure test case * more pr cleanups * more pr cleanups * refactor project_preferences_controller * houndbot refactorings * houndbot refactorings * add new test case for UPP read settings * text change on UPP spec --- .../api/v1/project_preferences_controller.rb | 56 ++++++++++++------- app/models/project.rb | 1 + config/routes.rb | 1 + .../v1/project_preferences_controller_spec.rb | 52 +++++++++++++++++ 4 files changed, 90 insertions(+), 20 deletions(-) diff --git a/app/controllers/api/v1/project_preferences_controller.rb b/app/controllers/api/v1/project_preferences_controller.rb index 8a0b49b24..476548c96 100644 --- a/app/controllers/api/v1/project_preferences_controller.rb +++ b/app/controllers/api/v1/project_preferences_controller.rb @@ -6,38 +6,54 @@ 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_for_update_settings, only: [:update_settings] + before_action :find_upp, only: %i[update_settings read_settings] - def update_settings + def read_settings skip_policy_scope - @upp.settings.merge! params_for[:settings] - @upp.save! - update_settings_response - end - - private - - def find_upp_for_update_settings - @upp = UserProjectPreference.find_by!( - user_id: params_for[:user_id], - project_id: params_for[:project_id] + render( + status: :ok, + json_api: serializer.page( + params, + @upp_list, + context + ) ) - raise Api::Unauthorized, 'You must be the project owner or a collaborator' unless user_allowed? - end - - def user_allowed? - @upp.project.owners_and_collaborators.include?(api_user.user) || api_user.is_admin? end - def update_settings_response + def update_settings + skip_policy_scope + @upp.settings.merge! params_for[:settings] + @upp.save! response.headers['Last-Modified'] = @upp.updated_at.httpdate + render( status: :ok, json_api: serializer.resource( {}, - UserProjectPreference.where(id: @upp.id), + @upp_list, 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 + end + + def find_upp + fetch_upp_list + @upp = @upp_list.first + raise ActiveRecord::RecordNotFound unless @upp.present? + + raise Api::Unauthorized, 'You must be the project owner or a collaborator' unless user_allowed? + end + + def user_allowed? + @upp.project.owners_and_collaborators.include?(api_user.user) || api_user.is_admin? + end end diff --git a/app/models/project.rb b/app/models/project.rb index 260d89943..8bc728587 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -15,6 +15,7 @@ class Project < ApplicationRecord has_many :tutorials has_many :field_guides, dependent: :destroy belongs_to :organization + has_many :user_project_preference, dependent: :destroy # uses the activated_state enum on the workflow has_many :workflows, -> { where(serialize_with_project: true).active}, diff --git a/config/routes.rb b/config/routes.rb index 40178522a..c09a16980 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -55,6 +55,7 @@ json_api_resources :project_preferences do collection do post :update_settings + get :read_settings end end diff --git a/spec/controllers/api/v1/project_preferences_controller_spec.rb b/spec/controllers/api/v1/project_preferences_controller_spec.rb index 68bbf6e7d..84f45d185 100644 --- a/spec/controllers/api/v1/project_preferences_controller_spec.rb +++ b/spec/controllers/api/v1/project_preferences_controller_spec.rb @@ -197,4 +197,56 @@ end end end + + describe '#read_settings' do + let!(:second_authorized_user) { create(:user) } + let!(:project) { create(:project, owner: authorized_user) } + let!(:first_upp) { create(:user_project_preference, project: project, user_id: authorized_user.id) } + let!(:second_upp) { create(:user_project_preference, project: project, user_id: second_authorized_user.id) } + let(:run_generic_read) { get :read_settings, params: { project_id: project.id, format: :json } } + let(:run_invalid_project) { get :read_settings, params: { project_id: 500, format: :json } } + let(:unauthorised_user) { create(:user) } + + describe 'generic preferences' do + before do + default_request user_id: authorized_user.id, scopes: scopes + run_generic_read + end + + it 'responds with a 200' do + expect(response.status).to eq(200) + end + + it 'returns the correct response data' do + json_response = JSON.parse(response.body) + expect(json_response['project_preferences'].count).to eq(2) + end + end + + describe 'invalid project' do + before do + default_request user_id: authorized_user.id, scopes: scopes + run_invalid_project + end + + it 'responds with a 404' do + expect(response.status).to eq(404) + end + end + + describe 'user specific preferences' do + 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) + end + + it 'only fetches settings of the specified user' do + default_request user_id: authorized_user.id, scopes: scopes + get :read_settings, params: { project_id: project.id, user_id: authorized_user.id, format: :json } + json_response = JSON.parse(response.body) + expect(json_response['project_preferences'].count).to eq(1) + end + end + end end