From 8b6a0eab0f30d89493d611d404ddb217386f6350 Mon Sep 17 00:00:00 2001
From: Vokhmin Alexey V <avokhmin@gmail.com>
Date: Wed, 3 Apr 2013 01:59:33 +0400
Subject: [PATCH] #51: some refactoring

---
 app/controllers/projects/issues_controller.rb       | 13 ++++---------
 .../projects/pull_requests_controller.rb            |  2 ++
 app/models/issue.rb                                 | 13 +++++++++++++
 3 files changed, 19 insertions(+), 9 deletions(-)

diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb
index 52c951d01..c5a9495ec 100644
--- a/app/controllers/projects/issues_controller.rb
+++ b/app/controllers/projects/issues_controller.rb
@@ -41,10 +41,7 @@ class Projects::IssuesController < Projects::BaseController
     @assignee_uname = params[:assignee_uname]
     @issue.user_id = current_user.id
 
-    unless can?(:write, @project)
-      @issue.assignee_id = nil
-      @issue.labelings = []
-    end
+    @issue.can_write_project = can?(:write, @project)
     if @issue.save
       @issue.subscribe_creator(current_user.id)
       flash[:notice] = I18n.t("flash.issue.saved")
@@ -60,11 +57,9 @@ class Projects::IssuesController < Projects::BaseController
   end
 
   def update
-    unless can?(:write, @project)
-      [:labelings, :labelings_attributes, :assignee_id].each{ |k| params[:issue].delete k }
-      params.delete :update_labels
-    end
-    @issue.labelings.destroy_all if params[:update_labels]
+    can_write_project = can?(:write, @project)
+    @issue.can_write_project = can_write_project
+    @issue.labelings.destroy_all if can_write_project && params[:update_labels]
     if params[:issue] && status = params[:issue][:status]
       @issue.set_close(current_user) if status == 'closed'
       @issue.set_open if status == 'open'
diff --git a/app/controllers/projects/pull_requests_controller.rb b/app/controllers/projects/pull_requests_controller.rb
index 5a4e7cbd9..055928aac 100644
--- a/app/controllers/projects/pull_requests_controller.rb
+++ b/app/controllers/projects/pull_requests_controller.rb
@@ -43,6 +43,7 @@ class Projects::PullRequestsController < Projects::BaseController
     @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
@@ -67,6 +68,7 @@ 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
diff --git a/app/models/issue.rb b/app/models/issue.rb
index 1d61be583..a247f475a 100644
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -13,6 +13,7 @@ class Issue < ActiveRecord::Base
   has_many :labels, :through => :labelings, :uniq => true
   has_one :pull_request, :dependent => :destroy
 
+  before_validation :sanitize_params_for_current_user
   validates :title, :body, :project_id, :presence => true
 
   after_create :set_serial_id
@@ -21,6 +22,7 @@ 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')
@@ -69,6 +71,17 @@ class Issue < ActiveRecord::Base
 
   protected
 
+  def sanitize_params_for_current_user
+    return true if can_write_project
+    if persisted?
+      self.assignee_id  = self.assignee_id
+      self.labelings    = self.labelings
+    else
+      self.assignee_id  = nil
+      self.labelings    = []
+    end
+  end
+
   def set_serial_id
     self.serial_id = self.project.issues.count
     self.save!