From 0a6c4879950a97e15d6cd9b43d67d7fd3abc366d Mon Sep 17 00:00:00 2001 From: Vokhmin Alexey V Date: Wed, 29 Apr 2015 02:26:03 +0300 Subject: [PATCH] #472: Updated the Projects::IssuesController and Api::V1::IssuesController --- app/controllers/api/v1/base_controller.rb | 2 +- app/controllers/api/v1/issues_controller.rb | 12 +++------- app/controllers/concerns/strong_params.rb | 4 ++-- app/controllers/projects/issues_controller.rb | 22 +++++++------------ app/models/issue.rb | 2 +- app/policies/issue_policy.rb | 13 +++++++++++ .../projects/issues_controller_spec.rb | 2 +- 7 files changed, 29 insertions(+), 28 deletions(-) diff --git a/app/controllers/api/v1/base_controller.rb b/app/controllers/api/v1/base_controller.rb index 5d25ef9f9..a40bedfe0 100644 --- a/app/controllers/api/v1/base_controller.rb +++ b/app/controllers/api/v1/base_controller.rb @@ -85,7 +85,7 @@ class Api::V1::BaseController < ApplicationController def update_subject(subject) authorize subject, :update? class_name = subject.class.name - if subject.update_attributes(subject_params(subject.class)) + if subject.update_attributes(subject_params(subject.class, subject)) render_json_response subject, "#{class_name} has been updated successfully" else render_validation_error subject, "#{class_name} has not been updated" diff --git a/app/controllers/api/v1/issues_controller.rb b/app/controllers/api/v1/issues_controller.rb index fb4fbc79d..4822f2800 100644 --- a/app/controllers/api/v1/issues_controller.rb +++ b/app/controllers/api/v1/issues_controller.rb @@ -44,20 +44,14 @@ class Api::V1::IssuesController < Api::V1::BaseController end def create - @issue = @project.issues.new(params[:issue]) + @issue = @project.issues.new + @issue.assign_attributes subject_params(Issue, @issue) @issue.user = current_user - @issue.assignee = nil unless policy(@project).write? create_subject @issue end def update - unless policy(@project).write? - params.delete :update_labels - [:assignee_id, :labelings, :labelings_attributes].each do |k| - params[:issue].delete k - end if params[:issue] - end - @issue.labelings.destroy_all if params[:update_labels] + @issue.labelings.destroy_all if params[:update_labels] && policy(@project).write? if params[:issue] && status = params[:issue].delete(:status) @issue.set_close(current_user) if status == 'closed' @issue.set_open if status == 'open' diff --git a/app/controllers/concerns/strong_params.rb b/app/controllers/concerns/strong_params.rb index 1a5870e17..4b7a6629e 100644 --- a/app/controllers/concerns/strong_params.rb +++ b/app/controllers/concerns/strong_params.rb @@ -10,7 +10,7 @@ module StrongParams end - def subject_params(subject_class) - permit_params(subject_class.name.underscore.to_sym, *policy(subject_class).permitted_attributes) + def subject_params(subject_class, subject = nil) + permit_params(subject_class.name.underscore.to_sym, *policy(subject || subject_class).permitted_attributes) end end diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index 006bf947a..544effc5a 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/app/controllers/projects/issues_controller.rb @@ -79,13 +79,10 @@ class Projects::IssuesController < Projects::BaseController end def create - @issue = @project.issues.build(params[:issue]) - @issue.user_id = current_user.id + @issue = @project.issues.new + @issue.assign_attributes(issue_params) + @issue.user = current_user - unless policy(@project).write? - @issue.assignee_id = nil - @issue.labelings = [] - end authorize @issue if @issue.save @issue.subscribe_creator(current_user.id) @@ -108,19 +105,12 @@ class Projects::IssuesController < Projects::BaseController format.json { status = 200 - unless policy(@project).write? - params.delete :update_labels - [:assignee_id, :labelings, :labelings_attributes].each do |k| - params[:issue].delete k - end if params[:issue] - end - if params[:issue] && status = params[:issue][:status] @issue.set_close(current_user) if status == 'closed' @issue.set_open if status == 'open' status = @issue.save ? 200 : 500 else - status = 422 unless @issue.update_attributes(params[:issue]) + status = 422 unless @issue.update_attributes(issue_params) end render status: status } @@ -169,6 +159,10 @@ class Projects::IssuesController < Projects::BaseController private + def issue_params + subject_params(Issue, @issue) + end + # Private: before_action hook which loads Issue. def load_issue authorize @issue = @project.issues.find_by!(serial_id: params[:id]) diff --git a/app/models/issue.rb b/app/models/issue.rb index c39174660..3ce99e372 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -49,7 +49,7 @@ class Issue < ActiveRecord::Base # attr_accessible :labelings_attributes, :title, :body, :assignee_id accepts_nested_attributes_for :labelings, - reject_if: lambda {|attributes| attributes['label_id'].blank?}, + reject_if: -> (attributes) { attributes['label_id'].blank? }, allow_destroy: true scope :opened, -> { where(status: [STATUS_OPEN, STATUS_REOPEN]) } diff --git a/app/policies/issue_policy.rb b/app/policies/issue_policy.rb index 34a0f0da1..715ba0392 100644 --- a/app/policies/issue_policy.rb +++ b/app/policies/issue_policy.rb @@ -18,4 +18,17 @@ class IssuePolicy < ApplicationPolicy is_admin? || record.user_id == user.id || local_admin?(record.project) end + # Public: Get list of parameters that the user is allowed to alter. + # + # Returns Array + def permitted_attributes + pa = %i(title body) + if ProjectPolicy.new(user, record.project).write? + pa << :assignee_id + pa << { labelings_attributes: %i(name color label_id) } + pa << { labelings: [] } + end + pa + end + end diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb index 8b63f5cb4..b39bb87b5 100644 --- a/spec/controllers/projects/issues_controller_spec.rb +++ b/spec/controllers/projects/issues_controller_spec.rb @@ -21,7 +21,7 @@ shared_context "issues controller" do issue: { title: "issue1", body: "issue body", - labelings_attributes: { @label.id => { label_id: @label.id }}, + labelings_attributes: { @label.id.to_s => { label_id: @label.id }}, assignee_id: @issue_user.id } }