refactor project_preferences_controller

app/controllers/api/v1/project_preferences_controller.rb

@@ -6,58 +6,48 @@ 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_project, only: [:read_settings]
+  before_action :find_upp, only: [:update_settings, :read_settings]
 
   def read_settings
     skip_policy_scope
-    read_and_update_settings_response
+    render(
+      status: :ok,
+      json_api: serializer.page(
+        params,
+        @upp_list,
+        context
+      )
+    )
   end
 
   def update_settings
     skip_policy_scope
     @upp.settings.merge! params_for[:settings]
     @upp.save!
-    read_and_update_settings_response
-  end
-
-  def find_project
-    @project = Project.find(params[:project_id])
-  end
+    response.headers['Last-Modified'] = @upp.updated_at.httpdate
 
-  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.resource(
+        {},
+        @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 find_upp
+    @upp_list = action_name == 'read_settings' ? UserProjectPreference.where(project_id: params[:project_id], email_communication: !nil) : UserProjectPreference.where(user_id: params_for[:user_id], project_id: params_for[:project_id])
 
-  def read_and_update_settings_response
-    set_last_modified_header if action_name == 'update_settings'
+    @upp = @upp_list.first
+    raise ActiveRecord::RecordNotFound unless @upp.present?
 
-    render_json_response
-  end
+    @upp_list = action_name == 'read_settings' && params[:user_id].present? ? @upp_list.where(user_id: params[:user_id]) : @upp_list
 
-  def set_last_modified_header
-    response.headers['Last-Modified'] = @upp.updated_at.httpdate
+    raise Api::Unauthorized, 'You must be the project owner or a collaborator' unless user_allowed?
   end
 
-  def render_json_response
-    if action_name == 'update_settings'
-      preferences = UserProjectPreference.where(id: @upp.id)
-    else
-      preferences = @project.user_project_preference.where.not(email_communication: nil)
-      preferences = params[:user_id].present? ? preferences.where(user_id: params[:user_id]) : preferences
-    end  
-
-    render(
-      status: :ok,
-      json_api: serializer.resource({}, preferences, context)
-    )
+  def user_allowed?
+    @upp.project.owners_and_collaborators.include?(api_user.user) || api_user.is_admin?
   end
 end

spec/controllers/api/v1/project_preferences_controller_spec.rb

@@ -239,13 +239,8 @@
         run_unauthorised_user_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(0)
+      it 'only fetches settings of owned project' do
+        expect(response.status).to eq(403)
       end
     end
   end