[webui][api] Fix the comment test/fixtures and the resulting fallout

src/api/app/controllers/application_controller.rb

@@ -482,15 +482,15 @@ def user
   def required_parameters(*parameters)
     parameters.each do |parameter|
       unless params.include? parameter.to_s
-        raise MissingParameterError, "Required Parameter #{parameter} missing"
+        raise MissingParameterError, "Required parameter #{parameter} missing."
       end
     end
   end
 
   def required_fields(*parameters)
     parameters.each do |parameter|
       if params[parameter].blank? 
-        raise NoDataEntered.new "Required Parameter #{parameter} is empty."
+        raise NoDataEntered.new "Required parameter #{parameter} is empty."
       end
     end
   end

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

@@ -21,7 +21,7 @@ def projects
 	end
 
 	def requests
-		comments = CommentRequest.where(bs_request_id: params[:bid])
+		comments = CommentRequest.where(bs_request_id: params[:id])
                 comment_thread = sort_comments(comments)
 		render :json => comment_thread
 	end
@@ -30,8 +30,6 @@ def packages_new
 		required_parameters :title, :body, :project, :package
 		required_fields :body, :title
 
-		permission_check!(params)
-
 		CommentPackage.save(params)
 		render_ok
 	end
@@ -40,18 +38,14 @@ def projects_new
 		required_parameters :title, :body, :project
 		required_fields :body, :title
 
-		permission_check!(params)
-
 		CommentProject.save(params)
 		render_ok
 	end
 
 	def requests_new
-		required_parameters :title, :body, :bs_request_id
+		required_parameters :title, :body, :id
 		required_fields :body, :title
 
-		permission_check!(params)
-
 		CommentRequest.save(params)
 		render_ok
 	end
@@ -70,10 +64,33 @@ def require_user
 	private
 
 	def permission_check!(params)
+		delete = false
+		comment = Comment.find(params[:comment_id])
 		package = Package.get_by_project_and_name(params[:project], params[:package]) if params[:package]    
 		project = Project.get_by_name(params[:project]) if params[:project]
+		request = BsRequest.find(params[:id]) if params[:id]
 
-		unless @http_user.login == params[:user] || @http_user.is_admin? || @http_user.has_local_permission?("change_project", project) || @http_user.has_local_permission?("change_package", package)
+		# Users can always delete their own comments
+		if @http_user.login == comment.user
+			delete = true
+		end
+		# Admins can always delete all comments
+		if @http_user.is_admin?
+			delete = true
+		end
+		# If you can change the project, you can delete the comment
+		if project and @http_user.has_local_permission?("change_project", project)
+			delete = true
+		end
+		# If you can change the package, you can delete the comment
+		if package and @http_user.has_local_permission?("change_package", package)
+			delete = true
+		end
+		# If you can review or if you are maintainer of the target of the request, you can delete the comment
+		if request and ( request.is_reviewer?(@http_user) || request.is_target_maintainer?(@http_user) )
+			delete = true
+		end
+		unless delete
 			raise CommentNoPermission.new, "You don't have the permissions to modify the content."
 		end
 	end

src/api/app/models/comment_request.rb

@@ -1,6 +1,7 @@
 class CommentRequest < Comment
 	def self.save(params)
 		super
+                @comment['bs_request_id'] = params[:id]
 		CommentRequest.create(@comment)
 	end
 end

src/api/test/fixtures/comments.yml

@@ -1,7 +1,7 @@
 project_comment_parent:
  id: 100
- title: Hurray
- body: I am making a comment
+ title: This project rules!
+ body: I am making a comment on a project.
  project_id: 2042
  package_id:
  bs_request_id:
@@ -11,41 +11,106 @@ project_comment_parent:
  created_at: 2013-07-25 14:00:21.000000000 Z
  updated_at: 2013-07-25 14:00:21.000000000 Z
 
-project_comment_reply:
+project_comment_reply_1:
  id: 101
- title:
- body: I am making a reply
+ title: This project rules!
+ body: And I like that comment.
  project_id: 2042
  package_id:
  bs_request_id:
  parent_id: 100
  type: CommentProject
- user: Admin
+ user: tom
  created_at: 2013-07-25 15:00:21.000000000 Z
  updated_at: 2013-07-25 15:00:21.000000000 Z
 
-package_comment_parent:
+project_comment_reply_2:
  id: 102
- title: Hurray 2
- body: I am making a comment 2
+ title: This project rules!
+ body: While I don't!
+ project_id: 2042
+ package_id:
+ bs_request_id:
+ parent_id: 101
+ type: CommentProject
+ user: adrian
+ created_at: 2013-07-25 15:47:21.000000000 Z
+ updated_at: 2013-07-25 15:47:21.000000000 Z
+
+package_comment_parent:
+ id: 200
+ title: This package rules!
+ body: I am making a comment on a package.
  project_id:
  package_id: 10094
  bs_request_id:
  parent_id:
  type: CommentPackage
+ user: Admin
+ created_at: 2013-07-25 14:05:30.000000000 Z
+ updated_at: 2013-07-25 14:05:30.000000000 Z
+
+package_comment_reply_1:
+ id: 201
+ title: This package rules!
+ body: And I like that comment.
+ project_id:
+ package_id: 10094
+ bs_request_id:
+ parent_id: 200
+ type: CommentPackage
  user: tom
- created_at: 2013-07-25 14:00:21.000000000 Z
- updated_at: 2013-07-25 14:00:21.000000000 Z
+ created_at: 2013-07-25 14:07:00.000000000 Z
+ updated_at: 2013-07-25 14:07:00.000000000 Z
+
+package_comment_reply_2:
+ id: 202
+ title: This package rules!
+ body: While I don't!
+ project_id:
+ package_id: 10094
+ bs_request_id:
+ parent_id: 201
+ type: CommentPackage
+ user: adrian
+ created_at: 2013-07-25 14:38:44.000000000 Z
+ updated_at: 2013-07-25 14:38:44.000000000 Z
 
 request_comment_parent:
- id: 103
- title: Hurray 3
- body: I am making a comment 3
+ id: 300
+ title: This request rules!
+ body: I am making a comment on a request.
  project_id:
  package_id:
  bs_request_id: 1000
  parent_id:
  type: CommentRequest
+ user: Admin
+ created_at: 2013-07-25 16:00:21.000000000 Z
+ updated_at: 2013-07-25 16:00:21.000000000 Z
+
+request_comment_reply_1:
+ id: 301
+ title: This request rules!
+ body: And I like that comment.
+ project_id:
+ package_id:
+ bs_request_id: 1000
+ parent_id: 300
+ type: CommentRequest
  user: tom
- created_at: 2013-07-25 14:00:21.000000000 Z
- updated_at: 2013-07-25 14:00:21.000000000 Z
+ created_at: 2013-07-25 17:00:21.000000000 Z
+ updated_at: 2013-07-25 17:00:21.000000000 Z
+
+request_comment_reply_2:
+ id: 302
+ title: This request rules!
+ body: While I don't!
+ project_id:
+ package_id:
+ bs_request_id: 1000
+ parent_id: 301
+ type: CommentRequest
+ user: adrian
+ created_at: 2013-07-25 18:00:21.000000000 Z
+ updated_at: 2013-07-25 18:00:21.000000000 Z

src/api/test/functional/comments_controller_test.rb

@@ -5,90 +5,101 @@ class CommentsControllerTest < ActionDispatch::IntegrationTest
   fixtures :all
 
   def setup
-   prepare_request_with_user("Admin","opensuse")
+    prepare_request_with_user("king","sunflower")
   end
 
-  def test_show_and_post_comments_on_project
+  def test_writing_project_comments
     # Testing new comment creation
-    post "/webui/comments/project/BaseDistro/new", {:project => "BaseDistro", :title => "This is a title", :body => "This is a body", :user => "Admin"}
+    post "/webui/comments/project/BaseDistro2.0/new", {:title => "This is a title", :body => "This is a body"}
     assert_response :success
 
-    # testing empty comments
-    post "/webui/comments/project/BaseDistro/new", {:project => "BaseDistro", :title => "This is a title", :body => "", :user => "Admin"}
+    # Empty title or body shouldn't work
+    post "/webui/comments/project/BaseDistro2.0/new", {:title => "", :body => "This is a body"}
     assert_response 403
+    post "/webui/comments/project/BaseDistro2.0/new", {:title => "This is a title", :body => ""}
+    assert_response 403
+  end
 
-    # counter test
-    get "/webui/comments/project/BaseDistro"
+  def test_writing_package_comments
+    # Testing new comment creation
+    post "/webui/comments/package/BaseDistro2.0/pack2/new", {:title => "This is a title", :body => "This is a body"}
     assert_response :success
 
-    post "/webui/comments/project/BaseDistro/new", {:project => "BaseDistro", :title => "This is a title"}
-    assert_response 400
+    # Empty title or body shouldn't work
+    post "/webui/comments/package/BaseDistro2.0/pack2/new", {:title => "", :body => "This is a body"}
+    assert_response 403
+    post "/webui/comments/package/BaseDistro2.0/pack2/new", {:title => "This is a title", :body => ""}
+    assert_response 403
   end
 
-  def test_update_permissions_for_comments_on_project
-    reset_auth
-    prepare_request_with_user "tom", "thunder"
-
-    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", :body => "Comment deleted"}
-    assert_response 400
+  def test_writing_request_comments
+    # Testing new comment creation
+    post "/webui/comments/request/998/new", {:title => "This is a title", :body => "This is a body"}
+    assert_response :success
 
-    # Test to see check permission on editing comments
+    # Empty title or body shouldn't work
+    post "/webui/comments/request/998/new", {:title => "", :body => "This is a body"}
+    assert_response 403
+    post "/webui/comments/request/998/new", {:title => "This is a title", :body => ""}
+    assert_response 403
+  end
 
-    put "/webui/comments/project/BaseDistro/edit", {:comment_id => 100, :user => 'Iggy',:project => "BaseDistro", :body => "Hurray this is a comment"}
-    assert_response 400
+  def test_reading_project_comments
+    # Getting comments
+    get "/webui/comments/project/BaseDistro"
+    assert_response :success
+  end
 
-    put "/webui/comments/project/BaseDistro/edit", {:comment_id => 100, :user => 'tom',:project => "BaseDistro", :body => "Hurray this is a comment 2"}
+  def test_deleting_project_comments
+    # Admins should be able to delete all comments
+    post "/webui/comments/project/BaseDistro/delete", {:comment_id => 102}
     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/delete", {:comment_id => 102, :user => 'tom', :body => "Comment deleted"}
+    # Users should be able to delete their own comments
+    post "/webui/comments/project/BaseDistro/delete", {:comment_id => 101}
     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', :body => "Comment deleted"}
+    # Users shouldn't be able to delete a comment they are not associated with
+    post "/webui/comments/project/BaseDistro/delete", {:comment_id => 100}
     assert_response 400
 
-    # Test to see check permission on editing comments
-
-    put "/webui/comments/package/BaseDistro/pack1/edit", {:comment_id => 102, :user => 'Iggy', :body => "Some comment"}
-    assert_response 400
+  end
 
-    put "/webui/comments/package/BaseDistro/pack1/edit", {:comment_id => 102, :user => 'tom', :body => "Some comment from the dark knight"}
+  def test_deleting_package_comments
+    # Admins should be able to delete all comments
+    post "/webui/comments/package/BaseDistro/pack1/delete", {:comment_id => 202}
     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/delete", {:comment_id => 103, :user => 'tom', :body => "Comment deleted"}
+
+    # Users should be able to delete their own comments
+    post "/webui/comments/package/BaseDistro/pack1/delete", {:comment_id => 201}
     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', :body => "Comment deleted"}
-    assert_response 400
+    # Users shouldn't be able to delete a comment they are not associated with
+    put "/webui/comments/package/BaseDistro/pack1/delete", {:comment_id => 200}
+    assert_response 404
+  end
 
-    # Test to see check permission on editing comments
+  def test_delete_request_comments
+    # Admins should be able to delete all comments
+    post "/webui/comments/request/1000/delete", {:comment_id => 302}
+    assert_response 200
 
-    put "/webui/comments/request/1000/edit", {:comment_id => 103, :user => 'Iggy', :body => "Comment from the president"}
-    assert_response 400
+    reset_auth
+    prepare_request_with_user "tom", "thunder"
 
-    put "/webui/comments/request/1000/edit", {:comment_id => 103, :user => 'tom', :body => "Comment from anony"}
+    # Users should be able to delete their own comments
+    post "/webui/comments/request/1000/delete", {:comment_id => 301}
     assert_response 200
+
+    # Users shouldn't be able to delete a comment they are not associated with
+    put "/webui/comments/request/1000/delete", {:comment_id => 300}
+    assert_response 404
   end
 
 end

src/webui/app/controllers/package_controller.rb

@@ -52,7 +52,6 @@ def show
 
     begin
       @comments = ApiDetails.read(:comments_by_package, @project, @package)
-      @comment_permission = @user.has_role?('maintainer', @project, @package) || @user.is_admin?
     rescue ActiveXML::Transport::Error => e
       render :text => e.summary, :status => 404, :content_type => "text/plain"
     end

src/webui/app/controllers/project_controller.rb

@@ -281,7 +281,6 @@ def show
     end
     begin
       @comments = ApiDetails.read(:comments_by_project, @project)
-      @comment_permission = @user.has_role?('maintainer', @project, @package) || @user.is_admin?
     rescue ActiveXML::Transport::Error => e
       render :text => e.summary, :status => 404, :content_type => "text/plain"
     end