#51: move logic to controller

app/controllers/projects/issues_controller.rb

@@ -41,7 +41,10 @@ class Projects::IssuesController < Projects::BaseController
     @assignee_uname = params[:assignee_uname]
     @issue.user_id = current_user.id
 
-    @issue.can_write_project = can?(:write, @project)
+    unless can?(:write, @project)
+      @issue.assignee_id  = nil
+      @issue.labelings    = []
+    end
     if @issue.save
       @issue.subscribe_creator(current_user.id)
       flash[:notice] = I18n.t("flash.issue.saved")
@@ -57,9 +60,13 @@ class Projects::IssuesController < Projects::BaseController
   end
 
   def update
-    can_write_project = can?(:write, @project)
+    unless can?(:write, @project)
-    @issue.can_write_project = can_write_project
+      params.delete :update_labels
-    @issue.labelings.destroy_all if can_write_project && params[: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]
     if params[:issue] && status = params[:issue][:status]
       @issue.set_close(current_user) if status == 'closed'
       @issue.set_open if status == 'open'

app/controllers/projects/pull_requests_controller.rb

@@ -38,12 +38,11 @@ class Projects::PullRequestsController < Projects::BaseController
     authorize! :read, to_project
 
     @pull = to_project.pull_requests.new pull_params
-    @pull.issue.assignee_id = (params[:issue] || {})[:assignee_id]
+    @pull.issue.assignee_id = (params[:issue] || {})[:assignee_id] if can?(:write, to_project)
     @pull.issue.user, @pull.issue.project, @pull.from_project = current_user, to_project, @project
     @pull.from_project_owner_uname = @pull.from_project.owner.uname
     @pull.from_project_name = @pull.from_project.name
 
-    @pull.issue.can_write_project = can?(:write, @project)
     if @pull.valid? # FIXME more clean/clever logics
       @pull.save # set pull id
       @pull.check(false) # don't make event transaction
@@ -68,7 +67,6 @@ class Projects::PullRequestsController < Projects::BaseController
   end
 
   def update
-    @pull.issue.can_write_project = can?(:write, @project)
     if (action = params[:pull_request_action]) && %w(close reopen).include?(params[:pull_request_action])
       if @pull.send("can_#{action}?")
         @pull.set_user_and_time current_user

app/models/issue.rb

@@ -13,7 +13,6 @@ class Issue < ActiveRecord::Base
   has_many :labels, :through => :labelings, :uniq => true
   has_one :pull_request, :dependent => :destroy
 
-  before_validation :sanitize_params
   validates :title, :body, :project_id, :presence => true
 
   after_create :set_serial_id
@@ -22,7 +21,6 @@ class Issue < ActiveRecord::Base
 
   attr_accessible :labelings_attributes, :title, :body, :assignee_id
   accepts_nested_attributes_for :labelings, :allow_destroy => true
-  attr_accessor :can_write_project
 
   scope :opened, where(:status => 'open')
   scope :closed, where(:status => 'closed')
@@ -71,17 +69,6 @@ class Issue < ActiveRecord::Base
 
   protected
 
-  def sanitize_params
-    return true if can_write_project
-    if persisted?
-      self.assignee_id  = self.assignee_id
-      self.labelings    = self.labelings.select{ |i| i.id }
-    else
-      self.assignee_id  = nil
-      self.labelings    = []
-    end
-  end
-
   def set_serial_id
     self.serial_id = self.project.issues.count
     self.save!