[webui][api] refactored permission check. Added condition for require…

…d params. Handingly unknown request ids for comment

src/api/app/controllers/webui/comments_controller.rb

@@ -38,32 +38,32 @@ def requests_new
 	end
 
 	def projects_edit
-		CommentProject.edit_comment(params)
+		CommentProject.edit(params)
 		render_ok
 	end
 
 	def packages_edit
-		CommentPackage.edit_comment(params)
+		CommentPackage.edit(params)
 		render_ok
 	end
 
 	def requests_edit
-		CommentRequest.edit_comment(params)
+		CommentRequest.edit(params)
 		render_ok
 	end
 
 	def projects_delete
-		CommentProject.delete_comment(params)
+		CommentProject.delete(params)
 		render_ok
 	end
 
 	def packages_delete
-		CommentPackage.delete_comment(params)
+		CommentPackage.delete(params)
 		render_ok
 	end
 
 	def requests_delete
-		CommentRequest.delete_comment(params)
+		CommentRequest.delete(params)
 		render_ok
 	end
 

src/api/app/models/comment.rb

@@ -3,14 +3,29 @@ class Comment < ActiveRecord::Base
   belongs_to :package
   belongs_to :bs_request
 
-  class NoDataEnteredError < APIException
-    setup 'no_data_entered', 403, "No data Entered"
+  class CommentNoDataEntered < APIException
+    setup 'comment_no_data_entered', 403, "No data Entered"
   end
-  class NoUserFound < APIException
-    setup 'no_user_found', 403, "No user found"
+  class CommentNoUserFound < APIException
+    setup 'comment_no_user_found', 403, "No user found"
   end
-  class WritePermissionError < APIException
-    setup "project_write_permission_error"
+  class CommentWritePermissionError < APIException
+    setup "comment_write_permission_error"
+  end
+
+  def self.fields_check!(params)
+    if params[:body].blank?
+      raise CommentNoDataEntered.new "You didn't add a body to the comment." 
+    elsif params[:user].blank?
+      raise CommentNoUserFound.new "No user found. Sign in before continuing."
+    end
+  end
+
+  def self.permission_check!(params)
+    unless User.current.login == params[:user] || User.current.is_admin? || @object_permission_check
+      raise CommentWritePermissionError, "You don't have the permissions to modify the content."
+    end
+    fields_check!(params)
   end
 
   def self.save(params)
@@ -20,34 +35,21 @@ def self.save(params)
   	@comment['user'] = params[:user]
   	@comment['parent_id'] = params[:parent_id] if params[:parent_id]
 
-    if @comment['body'].blank?
-      raise NoDataEnteredError.new "You didn't add a body to the comment." 
-    elsif !@comment['parent_id'] && @comment['title'].blank?
-      raise NoDataEnteredError.new "You didnt add a title to the comment"
-    elsif @comment['user'].blank?
-      raise NoUserFound.new "No user found. Sign in before continuing."
+    fields_check!(params)
+    if !params[:parent_id] && params[:title].blank?
+      raise CommentNoDataEntered.new "You didn't add a title to the comment." 
     end
   end
 
-  def self.edit_comment(params)
-
-    if User.current.login == params[:user]
-      self.update(params[:comment_id],:body => params[:body])
-    else
-      raise WritePermissionError, "You don't have the permissions to modify the content."
-    end
-
-    if params[:body].blank?
-      raise NoDataEnteredError.new "You didn't add a body to the comment." 
-    end
+  def self.edit(params)
+    permission_check!(params)
+    self.update(params[:comment_id],:body => params[:body])
   end
 
-  def self.delete_comment(params)
-    if @object_permission_check
-      self.update(params[:comment_id],:body => "Comment deleted.")
-    else
-      raise WritePermissionError, "You don't have the permissions to modify the content."
-    end
+  def self.delete(params)
+    params[:body] = "Comment deleted."
+    permission_check!(params)
+    self.update(params[:comment_id],:body => params[:body] , :user => params[:user])
   end
 
 end

src/api/app/models/comment_package.rb

@@ -6,9 +6,9 @@ def self.save(params)
 		CommentPackage.create(@comment)
 	end
 
-	def self.delete_comment(params)
+	def self.permission_check!(params)
 		package = Package.get_by_project_and_name(params[:project], params[:package])
-		@object_permission_check = (User.current.can_modify_package?(package) || User.current.is_admin? || User.current.login == params[:user])		
+		@object_permission_check = User.current.can_modify_package?(package)
 		super
 	end
 end

src/api/app/models/comment_project.rb

@@ -7,9 +7,9 @@ def self.save(params)
 		CommentProject.create(@comment)
 	end
 
-	def self.delete_comment(params)
+	def self.permission_check!(params)
 		project = Project.get_by_name(params[:project])
-		@object_permission_check = (User.current.can_modify_project?(project) || User.current.is_admin? || User.current.login == params[:user])		
+		@object_permission_check = User.current.can_modify_project?(project)	
 		super
 	end
 end

src/api/app/models/comment_request.rb

@@ -4,9 +4,4 @@ def self.save(params)
 		@comment['bs_request_id'] = params[:request_id]
 		CommentRequest.create(@comment)
 	end
-
-	def self.delete_comment(params)
-		@object_permission_check = (User.current.is_admin? || User.current.login == params[:user])		
-		super
-	end
 end

src/api/config/routes.rb

@@ -355,9 +355,9 @@
       post 'comments/package/:project/:package/new' => 'comments#packages_new', constraints: cons
       post 'comments/request/:id/new' => 'comments#requests_new', constraints: cons
 
-      put 'comments/project/:project/update' => 'comments#projects_edit', constraints: cons
-      put 'comments/package/:project/:package/update' => 'comments#packages_edit', constraints: cons
-      put 'comments/request/:id/update' => 'comments#requests_edit', constraints: cons
+      put 'comments/project/:project/edit' => 'comments#projects_edit', constraints: cons
+      put 'comments/package/:project/:package/edit' => 'comments#packages_edit', constraints: cons
+      put 'comments/request/:id/edit' => 'comments#requests_edit', constraints: cons
 
       put 'comments/project/:project/delete' => 'comments#projects_delete', constraints: cons
       put 'comments/package/:project/:package/delete' => 'comments#packages_delete', constraints: cons

src/api/test/functional/comments_controller_test.rb

@@ -29,65 +29,65 @@ def test_update_permissions_for_comments_on_project
     reset_auth
     prepare_request_with_user "tom", "thunder"
 
-    put "/webui/comments/project/BaseDistro/update", {:comment_id => 100, :user => 'tom', :title => "This is a title", :body => "Comment deleted"}
+    put "/webui/comments/project/BaseDistro/delete", {:comment_id => 100, :user => 'tom', :body => "Comment deleted"}
     assert_response 200
 
     # Test to see if another user can delete a comment he/she is not associated with
     prepare_request_with_user "tom", "thunder"
 
-    put "/webui/comments/project/BaseDistro/delete", {:comment_id => 100, :user => 'Iggy',:project => "BaseDistro", :title => "This is a title", :body => "Comment deleted"}
+    put "/webui/comments/project/BaseDistro/delete", {:comment_id => 100, :user => 'Iggy',:project => "BaseDistro", :body => "Comment deleted"}
     assert_response 400
 
     # Test to see check permission on editing comments
 
-    put "/webui/comments/project/BaseDistro/update", {:comment_id => 100, :user => 'Iggy',:project => "BaseDistro", :title => "This is a title", :body => "Comment deleted"}
+    put "/webui/comments/project/BaseDistro/edit", {:comment_id => 100, :user => 'Iggy',:project => "BaseDistro", :body => "Hurray this is a comment"}
     assert_response 400
 
-    put "/webui/comments/project/BaseDistro/update", {:comment_id => 100, :user => 'tom',:project => "BaseDistro", :title => "This is a title", :body => "Comment deleted"}
+    put "/webui/comments/project/BaseDistro/edit", {:comment_id => 100, :user => 'tom',:project => "BaseDistro", :body => "Hurray this is a comment 2"}
     assert_response 200
   end
 
   def test_update_permissions_for_comments_on_package
     reset_auth
     prepare_request_with_user "tom", "thunder"
 
-    put "/webui/comments/package/BaseDistro/pack1/update", {:comment_id => 102, :user => 'tom', :title => "This is a title", :body => "Comment deleted"}
+    put "/webui/comments/package/BaseDistro/pack1/delete", {:comment_id => 102, :user => 'tom', :body => "Comment deleted"}
     assert_response 200
 
     # Test to see if another user can delete a comment he/she is not associated with
     prepare_request_with_user "tom", "thunder"
 
-    put "/webui/comments/package/BaseDistro/pack1/delete", {:comment_id => 102, :user => 'Iggy', :title => "This is a title", :body => "Comment deleted"}
+    put "/webui/comments/package/BaseDistro/pack1/delete", {:comment_id => 102, :user => 'Iggy', :body => "Comment deleted"}
     assert_response 400
 
     # Test to see check permission on editing comments
 
-    put "/webui/comments/package/BaseDistro/pack1/update", {:comment_id => 102, :user => 'Iggy', :title => "This is a title", :body => "Comment deleted"}
+    put "/webui/comments/package/BaseDistro/pack1/edit", {:comment_id => 102, :user => 'Iggy', :body => "Some comment"}
     assert_response 400
 
-    put "/webui/comments/package/BaseDistro/pack1/update", {:comment_id => 102, :user => 'tom', :title => "This is a title", :body => "Comment deleted"}
+    put "/webui/comments/package/BaseDistro/pack1/edit", {:comment_id => 102, :user => 'tom', :body => "Some comment from the dark knight"}
     assert_response 200
   end
 
   def test_update_permissions_for_comments_on_request
     reset_auth
     prepare_request_with_user "tom", "thunder"
 
-    put "/webui/comments/request/1000/update", {:comment_id => 103, :user => 'tom', :title => "This is a title", :body => "Comment deleted"}
+    put "/webui/comments/request/1000/delete", {:comment_id => 103, :user => 'tom', :body => "Comment deleted"}
     assert_response 200
 
     # Test to see if another user can delete a comment he/she is not associated with
     prepare_request_with_user "tom", "thunder"
 
-    put "/webui/comments/request/1000/delete", {:comment_id => 103, :user => 'Iggy', :title => "This is a title", :body => "Comment deleted"}
+    put "/webui/comments/request/1000/delete", {:comment_id => 103, :user => 'Iggy', :body => "Comment deleted"}
     assert_response 400
 
     # Test to see check permission on editing comments
 
-    put "/webui/comments/request/1000/update", {:comment_id => 103, :user => 'Iggy', :title => "This is a title", :body => "Comment deleted"}
+    put "/webui/comments/request/1000/edit", {:comment_id => 103, :user => 'Iggy', :body => "Comment from the president"}
     assert_response 400
 
-    put "/webui/comments/request/1000/update", {:comment_id => 103, :user => 'tom', :title => "This is a title", :body => "Comment deleted"}
+    put "/webui/comments/request/1000/edit", {:comment_id => 103, :user => 'tom', :body => "Comment from anony"}
     assert_response 200
   end
 

src/webui/app/controllers/package_controller.rb

@@ -1093,6 +1093,8 @@ def comments
   end
 
   def save_comments
+    required_parameters :project, :package, :user, :body
+    required_parameters :title if !params[:parent_id]
     begin
       params[:project] = @project.name
       params[:package] = @package.name
@@ -1112,6 +1114,7 @@ def save_comments
   end
 
   def edit_comments
+    required_parameters :project, :package, :comment_id
     begin
       unless params[:update] == 'true'
         params[:project] = @project.name
@@ -1136,6 +1139,7 @@ def edit_comments
   end
 
   def delete_comments
+    required_parameters :user, :comment_id
     begin
       params[:project] = @project.name
       params[:package] = @package.name

src/webui/app/controllers/project_controller.rb

@@ -1278,6 +1278,8 @@ def comments
   end
 
   def save_comments
+    required_parameters :project, :user, :body
+    required_parameters :title if !params[:parent_id]
     begin
       params[:project] = @project.name
       ApiDetails.save_comments(:save_comments_for_projects, params)
@@ -1296,6 +1298,7 @@ def save_comments
   end
 
   def edit_comments
+    required_parameters :project, :comment_id
     begin
       unless params[:update] == 'true'
         params[:project] = @project.name
@@ -1319,6 +1322,7 @@ def edit_comments
   end
 
   def delete_comments
+    required_parameters :user, :comment_id
     begin
       params[:project] = @project.name
       ApiDetails.update_comments(:delete_comments_for_projects, params)

src/webui/app/controllers/request_controller.rb

@@ -291,6 +291,14 @@ def set_incident
 
 
   def comments
+    # avoiding display of comment section for unnecessary request ids
+    begin
+      @req = ApiDetails.read(:request, params[:id])
+    rescue ApiDetails::NotFoundError
+      flash[:error] = "Can't find request #{params[:id]}"
+      redirect_back_or_to :controller => "home", :action => "requests" and return
+    end
+
     unless params[:reply] == 'true'
       @comment = ApiDetails.read(:comments_by_request, params[:id])
       @comments_as_thread = sort_comments(@comment)
@@ -300,6 +308,8 @@ def comments
   end
 
   def save_comments
+    required_parameters :id, :user, :body
+    required_parameters :title if !params[:parent_id]
     begin
       params[:request_id] = params[:id]
       ApiDetails.save_comments(:save_comments_for_requests, params)
@@ -318,6 +328,7 @@ def save_comments
   end
 
   def edit_comments
+    required_parameters :id, :comment_id
     begin
       unless params[:update] == 'true'
         params[:request_id] = params[:id]
@@ -341,6 +352,7 @@ def edit_comments
   end
 
   def delete_comments
+    required_parameters :user, :comment_id
     begin
       params[:request_id] = params[:id]
       ApiDetails.update_comments(:delete_comments_for_requests, params)

src/webui/app/models/api_details.rb

@@ -40,9 +40,9 @@ def self.save_comments(route_name, params)
   def self.update_comments(route_name, params)
     uri = "/webui/" +
     case route_name.to_sym
-      when :edit_comments_for_projects then "comments/project/#{params[:project]}/update"
-      when :edit_comments_for_packages then "comments/package/#{params[:project]}/#{params[:package]}/update"
-      when :edit_comments_for_requests then "comments/request/#{params[:request_id]}/update"
+      when :edit_comments_for_projects then "comments/project/#{params[:project]}/edit"
+      when :edit_comments_for_packages then "comments/package/#{params[:project]}/#{params[:package]}/edit"
+      when :edit_comments_for_requests then "comments/request/#{params[:request_id]}/edit"
 
       when :delete_comments_for_projects then "comments/project/#{params[:project]}/delete"
       when :delete_comments_for_packages then "comments/package/#{params[:project]}/#{params[:package]}/delete"

src/webui/app/views/package/comments.html.erb

@@ -5,7 +5,6 @@
 <div class="grid_16 alpha omega box box-shadow">
 	<%= render :partial => "package/tabs" %>
 	<h3><%= @pagetitle %></h3>
-	<%=render :partial => "shared/parent_comments"%>
 </div>
-
+<%= render :partial => "shared/parent_comments"%>
 <%=render :partial => "shared/new_comment"%>

src/webui/app/views/request/comments.html.erb

@@ -5,7 +5,6 @@
 <div class="grid_16 alpha omega box box-shadow">
 	<%= render :partial => "request/tabs" %>
 	<h3><%= @pagetitle %></h3>
-	<%= render :partial => "shared/parent_comments"%>
 </div>
-
+<%= render :partial => "shared/parent_comments"%>
 <%=render :partial => "shared/new_comment"%>