4248 ownercollab read permissions for project preferences (#4299)

* 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

* refactor project_preferences_controller

* remove duplicate and add empty line after guard clause

* add test case to check the data response on fetching read_settings

* merge relevant and irrelevant attributes tests on project_preferences_controller_spec

* merge relevant and irrelevant attributes tests on project_preferences_controller_spec

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

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