From 5e3addd5d2dbbdc91470170d685c442205019af7 Mon Sep 17 00:00:00 2001
From: Pavel Chipiga <pavel.chipiga@gmail.com>
Date: Wed, 28 Dec 2011 04:57:42 +0200
Subject: [PATCH] Great refactor for cancan abilities. Remove product
 relations. Fix git abilities. Fix and refactor specs. Refs #85. Refactor
 issues controller to avoid serial_id problem. Fix links and routes. Fix minor
 bugs and translations. Fix specs. Refs #54

---
 app/controllers/build_lists_controller.rb     |   2 +-
 app/controllers/comments_controller.rb        |   6 +-
 app/controllers/issues_controller.rb          |  32 +-
 app/controllers/platforms_controller.rb       |   4 +-
 app/models/ability.rb                         | 273 ++++++------------
 app/models/issue.rb                           |   4 +
 app/models/product.rb                         |  25 +-
 app/views/comments/_form.html.haml            |   2 +-
 app/views/issues/_list.html.haml              |   4 +-
 app/views/issues/show.html.haml               |   2 +-
 config/locales/ru.yml                         |   9 +-
 config/routes.rb                              |   4 +-
 lib/grack/base.rb                             |   2 +-
 .../build_lists_controller_spec.rb            |  10 +-
 spec/controllers/issues_controller_spec.rb    |  18 +-
 spec/controllers/products_controller_spec.rb  |   3 +-
 spec/controllers/projects_controller_spec.rb  |   5 +-
 spec/models/cancan_spec.rb                    |  43 +--
 .../shared_examples/projects_controller.rb    |   2 +-
 19 files changed, 176 insertions(+), 274 deletions(-)

diff --git a/app/controllers/build_lists_controller.rb b/app/controllers/build_lists_controller.rb
index cbd847933..44b1773f1 100644
--- a/app/controllers/build_lists_controller.rb
+++ b/app/controllers/build_lists_controller.rb
@@ -9,7 +9,7 @@ class BuildListsController < ApplicationController
   before_filter :find_build_list_by_bs, :only => [:publish_build, :status_build, :pre_build, :post_build]
 
   load_and_authorize_resource :project, :only => NESTED_ACTIONS
-  load_and_authorize_resource :through => :project, :only => NESTED_ACTIONS, :shallow => true
+  load_and_authorize_resource :build_list, :through => :project, :only => NESTED_ACTIONS, :shallow => true
   load_and_authorize_resource :except => CALLBACK_ACTIONS.concat(NESTED_ACTIONS)
 
   def index
diff --git a/app/controllers/comments_controller.rb b/app/controllers/comments_controller.rb
index 93baf3b77..e20aa3af1 100644
--- a/app/controllers/comments_controller.rb
+++ b/app/controllers/comments_controller.rb
@@ -18,7 +18,7 @@ class CommentsController < ApplicationController
       flash[:notice] = I18n.t("flash.comment.saved")
       redirect_to :back
     else
-      flash[:error] = I18n.t("flash.comment.saved_error")
+      flash[:error] = I18n.t("flash.comment.save_error")
       render :action => 'new'
     end
   end
@@ -32,9 +32,9 @@ class CommentsController < ApplicationController
     if @comment.update_attributes(params[:comment])
       flash[:notice] = I18n.t("flash.comment.saved")
       #redirect_to :back
-      redirect_to show_issue_path(@comment.commentable.project, @comment.commentable.serial_id)
+      redirect_to [@comment.commentable.project, @comment.commentable]
     else
-      flash[:error] = I18n.t("flash.comment.saved_error")
+      flash[:error] = I18n.t("flash.comment.save_error")
       render :action => 'new'
     end
   end
diff --git a/app/controllers/issues_controller.rb b/app/controllers/issues_controller.rb
index a6a4cb734..3f1a6f4ec 100644
--- a/app/controllers/issues_controller.rb
+++ b/app/controllers/issues_controller.rb
@@ -1,15 +1,14 @@
 class IssuesController < ApplicationController
   before_filter :authenticate_user!
-  before_filter :find_project, :except => [:destroy]
-  before_filter :find_and_authorize_by_serial_id, :only => [:show, :edit]
-  before_filter :set_issue_stub, :only => [:new, :create]
+  before_filter :find_project
+  before_filter :find_issue_by_serial_id, :only => [:show, :edit, :update, :destroy]
+
+  load_and_authorize_resource :project
+  load_and_authorize_resource :issue, :through => :project, :find_by => :serial_id
 
-  load_and_authorize_resource :except => [:show, :edit, :index]
-  authorize_resource :project, :only => [:index]
   autocomplete :user, :uname
 
   def index
-    @issues = Issue.scoped
     @issues = @project.issues
     case params[:status]
     when 'open'
@@ -20,6 +19,10 @@ class IssuesController < ApplicationController
     @issues = @issues.paginate :per_page => 10, :page => params[:page]
   end
 
+  def new
+    @issue = Issue.new(:project => @project)
+  end
+
   def create
     @user_id = params[:user_id]
     @user_uname = params[:user_uname]
@@ -27,11 +30,11 @@ class IssuesController < ApplicationController
     @issue = Issue.new(params[:issue])
     @issue.user_id = @user_id
     @issue.project_id = @project.id
-    if @issue.save!
+    if @issue.save
       flash[:notice] = I18n.t("flash.issue.saved")
       redirect_to project_issues_path(@project)
     else
-      flash[:error] = I18n.t("flash.issue.saved_error")
+      flash[:error] = I18n.t("flash.issue.save_error")
       render :action => :new
     end
   end
@@ -47,9 +50,9 @@ class IssuesController < ApplicationController
 
     if @issue.update_attributes( params[:issue].merge({:user_id => @user_id}) )
       flash[:notice] = I18n.t("flash.issue.saved")
-      redirect_to show_issue_path(@project, @issue.serial_id)
+      redirect_to [@project, @issue]
     else
-      flash[:error] = I18n.t("flash.issue.saved_error")
+      flash[:error] = I18n.t("flash.issue.save_error")
       render :action => :new
     end
   end
@@ -67,12 +70,7 @@ class IssuesController < ApplicationController
     @project = Project.find(params[:project_id])
   end
 
-  def find_and_authorize_by_serial_id
-    @issue = @project.issues.where(:serial_id => params[:serial_id])[0]
-    authorize! params[:action].to_sym, @issue
-  end
-
-  def set_issue_stub
-    @issue = Issue.new(:project => @project)
+  def find_issue_by_serial_id
+    @issue = @project.issues.find_by_serial_id!(params[:id])
   end
 end
diff --git a/app/controllers/platforms_controller.rb b/app/controllers/platforms_controller.rb
index 3aed37c95..3ce47f9a7 100644
--- a/app/controllers/platforms_controller.rb
+++ b/app/controllers/platforms_controller.rb
@@ -64,7 +64,7 @@ class PlatformsController < ApplicationController
       flash[:notice] = I18n.t("flash.platform.saved")
       redirect_to @platform
     else
-      flash[:error] = I18n.t("flash.platform.saved_error")
+      flash[:error] = I18n.t("flash.platform.save_error")
       render :action => :new
     end
   end
@@ -81,7 +81,7 @@ class PlatformsController < ApplicationController
       flash[:notice] = I18n.t("flash.platform.saved")
       redirect_to @platform
     else
-      flash[:error] = I18n.t("flash.platform.saved_error")
+      flash[:error] = I18n.t("flash.platform.save_error")
       render :action => :new
     end
   end
diff --git a/app/models/ability.rb b/app/models/ability.rb
index dfd6f105d..9a601f37e 100644
--- a/app/models/ability.rb
+++ b/app/models/ability.rb
@@ -1,178 +1,95 @@
+# If rules goes one by one CanCan joins them by 'OR' sql operator
+# If rule has multiple conditions CanCan joins them by 'AND' sql operator
+# WARNING:      
+# - put cannot rules _after_ can rules and not before!
+# - beware inner joins. Use sub queries against them! 
+
 class Ability
   include CanCan::Ability
 
   def initialize(user)
     user ||= User.new # guest user (not logged in)
+    @user = user
 
     if user.admin?
       can :manage, :all
     else
-      #WARNING:
-      # - put cannot rules _after_ can rules and not before!
-      # - beware inner joins. Use sub queries against them! 
       # Shared rights between guests and registered users
       can :forbidden, Platform
 
-      can :read, [Repository, Platform], :visibility => 'open'
       # TODO remove because auth callbacks skipped
       can :auto_build, Project
-      can [:status_build, :pre_build, :post_build, :circle_build, :new_bbdt], BuildList
+      can [:publish_build, :status_build, :pre_build, :post_build, :circle_build, :new_bbdt], BuildList
 
-      # Guest rights
-      if user.guest?
+      if user.guest? # Guest rights
         can :create, User
-      # Registered user rights
-      else
-        can [:read, :platforms], Category
+      else # Registered user rights
+        can [:show, :autocomplete_user_uname], User
+
+        can [:read, :create], Group
+        can [:update, :manage_members], Group do |group|
+          group.objects.exists?(:object_type => 'User', :object_id => user.id, :role => 'admin') # or group.owner_id = user.id
+        end
+        can :destroy, Group, :owner_id => user.id
+
+        can :create, Project
+        can :read, Project, :visibility => 'open'
+        can :read, Project, :owner_type => 'User', :owner_id => user.id
+        can :read, Project, :owner_type => 'Group', :owner_id => user.group_ids
+        can(:read, Project, read_relations_for('projects')) {|project| local_reader? project}
+        can(:write, Project) {|project| local_writer? project} # for grack
+        can([:update, :manage_collaborators], Project) {|project| local_admin? project}
+        can(:fork, Project) {|project| can? :read, project}
+        can(:destroy, Project) {|project| owner? project}
 
         can :create, AutoBuildList
         can [:index, :destroy], AutoBuildList, :project_id => user.own_project_ids
-        # If rules goes one by one CanCan joins them by 'OR' sql operator
-        can :read, Project, :visibility => 'open'
-        can :read, Group
-        can :read, User
-        cannot :index, User
-        can :manage_collaborators, Project do |project|
-          project.relations.exists? :object_id => user.id, :object_type => 'User', :role => 'admin'
-        end
-        can :manage_members, Group do |group|
-          group.objects.exists? :object_id => user.id, :object_type => 'User', :role => 'admin'
-        end
 
-        # Put here model names which objects can user create
-        can :create, Project
-        can :create, Group
-        can :publish, BuildList do |build_list|
-          build_list.can_publish? && build_list.project.relations.exists?(:object_type => 'User', :object_id => user.id)
-        end
-        can [:index, :read], BuildList, ["build_lists.project_id IN (SELECT id FROM projects WHERE visibility = ?)", 'open'] do |build_list|
-          build_list.project.public?
-        end
-        can [:index, :read], BuildList, build_lists_in_relations_with(:object_type => 'User', :object_id => user.id) do |build_list|
-          build_list.project.relations.exists?(:object_type => 'User', :object_id => user.id)
-        end
+        can :read, BuildList, :project => {:visibility => 'open'}
+        can :read, BuildList, :project => {:owner_type => 'User', :owner_id => user.id}
+        can :read, BuildList, :project => {:owner_type => 'Group', :owner_id => user.group_ids}
+        can(:read, BuildList, read_relations_for('build_lists', 'projects')) {|build_list| can? :read, build_list.project}
+        can(:create, BuildList) {|build_list| can? :write, build_list.project}
+        can(:publish, BuildList) {|build_list| build_list.can_publish? && can?(:write, build_list.project)}
+
+        can :read, Platform, :visibility => 'open'
+        can :read, Platform, :owner_type => 'User', :owner_id => user.id
+        can :read, Platform, :owner_type => 'Group', :owner_id => user.group_ids
+        can(:read, Platform, read_relations_for('platforms')) {|platform| local_reader? platform}
+        can(:update, Platform) {|platform| local_admin? platform}
+        can([:freeze, :unfreeze, :destroy], Platform) {|platform| owner? platform}
+        can :autocomplete_user_uname, Platform
+
+        # TODO delegate to platform?
+        can :read, Repository, :visibility => 'open'
+        can :read, Repository, :owner_type => 'User', :owner_id => user.id
+        can :read, Repository, :owner_type => 'Group', :owner_id => user.group_ids
+        can(:read, Repository, read_relations_for('repositories')) {|repository| local_reader? repository}
+        can(:create, Repository) {|repository| local_admin? repository.platform}
+        can([:update, :add_project, :remove_project], Repository) {|repository| local_admin? repository}
+        can([:change_visibility, :settings, :destroy], Repository) {|repository| owner? repository}
+
+        can :read, Product, :platform => {:owner_type => 'User', :owner_id => user.id}
+        can :read, Product, :platform => {:owner_type => 'Group', :owner_id => user.group_ids}
+        can(:manage, Product, read_relations_for('products', 'platforms')) {|product| local_admin? product.platform}
+
+        can [:read, :platforms], Category
 
         can [:read, :create], PrivateUser, :platform => {:owner_type => 'User', :owner_id => user.id}
-
-        # If rule has multiple conditions CanCan joins them by 'AND' sql operator
-        can [:read, :update, :destroy], Project, :owner_type => 'User', :owner_id => user.id
-        #can :read, Project, :relations => {:role => 'reader'}
-        can :read, Project, projects_in_relations_with(:role => 'reader', :object_type => 'User', :object_id => user.id) do |project|
-          #The can? and cannot? call cannot be used with a raw sql 'can' definition.
-          project.relations.exists?(:role => 'reader', :object_type => 'User', :object_id => user.id)
-        end
-        #can [:update], Project, :relations => {:role => 'writer'}
-        can [:read, :update], Project, projects_in_relations_with(:role => ['writer', 'admin'], :object_type => 'User', :object_id => user.id)  do |project|
-          project.relations.exists?(:role => ['writer', 'admin'], :object_type => 'User', :object_id => user.id)
-        end
-
-        can [:read, :update, :destroy], Product, products_in_relations_with(:role => ['writer', 'admin'], :object_type => 'User', :object_id => user.id)  do |product|
-          product.relations.exists?(:role => 'admin', :object_type => 'User', :object_id => user.id)
-        end
-        # Small CanCan hack by Product.new(:platform_id => ...)
-        can [:new, :create], Product do |product|
-          product.platform.relations.exists?(:role => 'admin', :object_type => 'User', :object_id => user.id)
-        end
-
-        can [:read, :update], Group, groups_in_relations_with(:role => ['writer', 'admin'], :object_type => 'User', :object_id => user.id) do |group|
-          group.objects.exists?(:role => ['writer', 'admin'], :object_type => 'User', :object_id => user.id)
-        end
-
-        can :manage, Platform, :owner_type => 'User', :owner_id => user.id
-        can :manage, Platform, platforms_in_relations_with(:role => 'admin', :object_type => 'User', :object_id => user.id) do |platform|
-          platform.relations.exists?(:role => 'admin', :object_type => 'User', :object_id => user.id)
-        end
-        #can :read, Platform, :members => {:id => user.id}
-        can :read, Platform, platforms_in_relations_with(:role => 'reader', :object_type => 'User', :object_id => user.id) do |platform|
-          platform.relations.exists?(:role => 'reader', :object_type => 'User', :object_id => user.id)
-        end
-
-        can [:manage, :add_project, :remove_project, :change_visibility, :settings], Repository, :owner_type => 'User', :owner_id => user.id
-        can :manage, Repository, repositories_in_relations_with(:role => 'admin', :object_type => 'User', :object_id => user.id) do |repository|
-          repository.relations.exists?(:role => 'admin', :object_type => 'User', :object_id => user.id)
-        end
-        #can :read, Repository, :members => {:id => user.id}
-        can :read, Repository, repositories_in_relations_with(:role => 'reader', :object_type => 'User', :object_id => user.id) do |repository|
-          repository.relations.exists?(:role => 'reader', :object_type => 'User', :object_id => user.id)
-        end
-        # Small CanCan hack by Repository.new(:platform_id => ...)
-        can [:new, :create], Repository do |repository|
-          repository.platform.relations.exists?(:role => 'admin', :object_type => 'User', :object_id => user.id)
-        end
-
-        can [:read, :index], Issue do |issue|
-          issue.status == 'open'
-        end
-        #can [:read], Issue, :status => 'open'
-        #can [:show], Issue, with_project_id_in_relations_with(:object_type => 'User', :object_id => user.id)
-        can [:read, :index], Issue do |issue|
-          issue.project.relations.exists?(:object_type => 'User', :object_id => user.id)
-        end
-        can [:create, :new], Issue do |issue|
-          issue.project.relations.exists?(:role => ['writer', 'admin'], :object_type => 'User', :object_id => user.id)
-        end
-        can [:edit, :update], Issue do |issue|
-          issue.user_id == user.id || (user.id == issue.project.owner_id && issue.project.owner_type == 'User') ||
-          issue.project.relations.exists?(:role => 'admin', :object_type => 'User', :object_id => user.id)
-        end
-        can [:create, :new], Comment do |comment|
-          comment.commentable.project.relations.exists?(:object_type => 'User', :object_id => user.id)
-        end
-        can [:edit, :update], Comment do |comment|
-          comment.user_id == user.id || (user.id == comment.commentable.project.owner_id && comment.commentable.project.owner_type == 'User') ||
-          comment.commentable.project.relations.exists?(:role => 'admin', :object_type => 'User', :object_id => user.id)
-        end
-        #
-        cannot [:index, :edit, :update, :create, :new, :read, :show], Issue do |issue|
-          !issue.project.has_issues
-        end
-        cannot [:edit, :update, :create, :new, :destroy], Comment do |comment|
-          !comment.commentable.project.has_issues
-        end
-
-        #can :read, Repository
-        # TODO: Add personal repos rules
-
-        # Same rights for groups:
         can [:read, :create], PrivateUser, :platform => {:owner_type => 'Group', :owner_id => user.group_ids}
-        can :publish, BuildList do |build_list|
-          build_list.can_publish? && build_list.project.relations.exists?(:object_type => 'Group', :object_id => user.group_ids)
-        end
-        can [:index, :read], BuildList, build_lists_in_relations_with(:object_type => 'Group', :object_id => user.group_ids) do |build_list|
-          build_list.project.relations.exists?(:object_type => 'Group', :object_id => user.group_ids)
-        end
 
-        can :manage_collaborators, Project, projects_in_relations_with(:role => 'admin', :object_type => 'Group', :object_id => user.group_ids) do |project|
-          project.relations.exists? :object_id => user.group_ids, :object_type => 'Group', :role => 'admin'
-        end
+        # can :read, Issue, :status => 'open'
+        can :read, Issue, :project => {:visibility => 'open'}
+        can :read, Issue, :project => {:owner_type => 'User', :owner_id => user.id}
+        can :read, Issue, :project => {:owner_type => 'Group', :owner_id => user.group_ids}
+        can(:read, Issue, read_relations_for('issues', 'projects')) {|issue| can? :read, issue.project rescue nil}
+        can(:create, Issue) {|issue| can? :write, issue.project}
+        can([:update, :destroy], Issue) {|issue| issue.user_id == user.id or local_admin?(issue.project)}
+        cannot :manage, Issue, :project => {:has_issues => false} # switch off issues
 
-        can [:read, :update, :destroy], Project, :owner_type => 'Group', :owner_id => user.group_ids
-        #can :read, Project, :relations => {:role => 'reader', :object_type => 'Group', :object_id => user.group_ids}
-        can :read, Project, projects_in_relations_with(:role => 'reader', :object_type => 'Group', :object_id => user.group_ids) do |project|
-          project.relations.exists?(:role => 'reader', :object_type => 'Group', :object_id => user.group_ids)
-        end
-        #can [:update], Project, :relations => {:role => 'writer', :object_type => 'Group', :object_id => user.group_ids}
-        can [:read, :update], Project, projects_in_relations_with(:role => ['writer', 'admin'], :object_type => 'Group', :object_id => user.group_ids) do |project|
-          project.relations.exists?(:role => ['writer', 'admin'], :object_type => 'Group', :object_id => user.group_ids)
-        end
-
-        can :manage, Platform, :owner_type => 'Group', :owner_id => user.group_ids
-        #can :read, Platform, :groups => {:id => user.group_ids}
-        can :read, Platform, platforms_in_relations_with(:role => 'reader', :object_type => 'Group', :object_id => user.group_ids) do |platform|
-          platform.relations.exists?(:role => 'reader', :object_type => 'Group', :object_id => user.group_ids)
-        end
-
-        can [:manage, :add_project, :remove_project], Repository, :owner_type => 'Group', :owner_id => user.group_ids
-        #can :read, Platform, :groups => {:id => user.group_ids}
-        can :read, Repository, repositories_in_relations_with(:role => 'reader', :object_type => 'Group', :object_id => user.group_ids) do |repository|
-          repository.relations.exists?(:role => 'reader', :object_type => 'Group', :object_id => user.group_ids)
-        end
-
-        can(:fork, Project) {|p| can? :read, p}
-        can(:create, BuildList) {|bl| can? :update, bl.project}
-
-        # Things that can not do simple user
-        cannot :create, [Platform, User]
+        can(:create, Comment) {|comment| can? :read, comment.commentable.project}
+        can(:update, Comment) {|comment| can? :update, comment.user_id == user.id or local_admin?(comment.commentable.project)}
+        cannot :manage, Comment, :commentable => {:project => {:has_issues => false}} # switch off issues
       end
     end
 
@@ -183,42 +100,34 @@ class Ability
     cannot :destroy, Issue
   end
 
-  # Sub query for platforms, projects relations
-  # TODO: Replace table names list by method_missing way
-  %w[platforms projects products repositories groups].each do |table_name|
-    define_method table_name + "_in_relations_with" do |opts|
-      query = "#{ table_name }.id IN (SELECT target_id FROM relations WHERE relations.target_type = '#{ table_name.singularize.camelize }'"
-      opts.each do |key, value|
-        query = query + " AND relations.#{ key } #{ value.class == Array ? 'IN (?)' : '= ?' } "
-      end
-      query = query + ")"
-
-      return opts.values.unshift query
-    end
+  # TODO group_ids ??
+  def read_relations_for(table, parent = nil)
+    key = parent ? "#{parent.singularize}_id" : 'id'
+    parent ||= table
+    ["#{table}.#{key} IN (
+      SELECT target_id FROM relations WHERE relations.target_type = ? AND
+      (relations.object_type = 'User' AND relations.object_id = ? OR
+       relations.object_type = 'Group' AND relations.object_id IN (?)))", parent.classify, @user, @user.group_ids]
   end
 
-  def with_project_id_in_relations_with(opts = {})
-    query = "issues.project_id IN (SELECT target_id FROM relations WHERE relations.target_type = 'issues'"
-    opts.each do |key, value|
-      query = query + " AND relations.#{ key } #{ value.class == Array ? 'IN (?)' : '= ?' } "
-    end
-    query = query + ")"
-
-    return opts.values.unshift query
+  def relation_exists_for(object, roles)
+    object.relations.exists?(:object_id => @user.id, :object_type => 'User', :role => roles) or
+    object.relations.exists?(:object_id => @user.group_ids, :object_type => 'Group', :role => roles)
   end
 
-  def build_lists_in_relations_with(opts)
-    query = "build_lists.project_id IN (SELECT target_id FROM relations WHERE relations.target_type = 'Project'"
-    opts.each do |key, value|
-      query = query + " AND relations.#{ key } #{ value.class == Array ? 'IN (?)' : '= ?' } "
-    end
-    query = query + ")"
-
-    return opts.values.unshift query
+  def local_reader?(object)
+    relation_exists_for(object, %w{reader writer admin}) or owner?(object)
   end
 
-  ## Sub query for project relations
-  #def projects_in_relations_with(opts={})
-  #  ["projects.id IN (SELECT target_id FROM relations WHERE relations.object_id #{ opts[:object_id].class == Array ? 'IN (?)' : '= ?' } AND relations.object_type = '#{ opts[:object_type] }' AND relations.target_type = 'Platform') AND relations.role", opts[:object_id]]
-  #end
+  def local_writer?(object)
+    relation_exists_for(object, %w{writer admin}) or owner?(object)
+  end
+
+  def local_admin?(object)
+    relation_exists_for(object, 'admin') or owner?(object)
+  end
+
+  def owner?(object)
+    object.owner == @user or @user.own_groups.include?(object.owner)
+  end
 end
diff --git a/app/models/issue.rb b/app/models/issue.rb
index ad6f57468..f320f3309 100644
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -12,6 +12,10 @@ class Issue < ActiveRecord::Base
 
   after_create :set_serial_id
 
+  def to_param
+    serial_id.to_s
+  end
+
   protected
 
   def set_serial_id
diff --git a/app/models/product.rb b/app/models/product.rb
index 637fdee90..36337f4f9 100644
--- a/app/models/product.rb
+++ b/app/models/product.rb
@@ -3,11 +3,9 @@ class Product < ActiveRecord::Base
 
   belongs_to :platform
   has_many :product_build_lists, :dependent => :destroy
-  has_many :relations, :as => :target, :dependent => :destroy
 
   after_validation :merge_tar_errors
   before_save :destroy_tar?
-  after_create :add_admin_relations
 
   has_attached_file :tar
 
@@ -69,21 +67,12 @@ class Product < ActiveRecord::Base
 
   protected
 
-    def destroy_tar?
-      self.tar.clear if @delete_tar == "1"
-    end
-
-    def merge_tar_errors
-      errors[:tar] += errors[:tar_content_type]
-      errors[:tar_content_type] = []
-    end
-
-    def add_admin_relations
-      platform.relations.where(:role => 'admin').each do |rel|
-        r = relations.build(:role => 'admin', :object_type => rel.object_type)
-        r.object_id = rel.object_id
-        r.save
-      end
-    end
+  def destroy_tar?
+    self.tar.clear if @delete_tar == "1"
+  end
 
+  def merge_tar_errors
+    errors[:tar] += errors[:tar_content_type]
+    errors[:tar_content_type] = []
+  end
 end
diff --git a/app/views/comments/_form.html.haml b/app/views/comments/_form.html.haml
index ccca4cc1d..eb5acaf4d 100644
--- a/app/views/comments/_form.html.haml
+++ b/app/views/comments/_form.html.haml
@@ -7,4 +7,4 @@
     = image_tag("web-app-theme/icons/tick.png", :alt => t("layout.save"))
     = t("layout.save")
   %span.text_button_padding= t("layout.or")
-  = link_to t("layout.cancel"), show_issue_path(@issue.project, @issue.serial_id), :class => "text_button_padding link_button"
+  = link_to t("layout.cancel"), [@issue.project, @issue], :class => "text_button_padding link_button"
diff --git a/app/views/issues/_list.html.haml b/app/views/issues/_list.html.haml
index dfb083664..98d5d5448 100644
--- a/app/views/issues/_list.html.haml
+++ b/app/views/issues/_list.html.haml
@@ -6,10 +6,10 @@
   - @issues.each do |issue|
     %tr{:class => cycle("odd", "even")}
       %td
-        = link_to issue.title, show_issue_path(@project, issue.serial_id)
+        = link_to issue.title, [@project, issue]
       %td
         = issue.status
       %td.last
-        = link_to t("layout.show"), show_issue_path(@project, issue.serial_id)
+        = link_to t("layout.show"), [@project, issue]
         |
         = link_to t("layout.delete"), project_issue_path(@project, issue), :method => :delete, :confirm => t("layout.issues.confirm_delete")  if can? :destroy, issue
diff --git a/app/views/issues/show.html.haml b/app/views/issues/show.html.haml
index 51c080e4a..a68e58fea 100644
--- a/app/views/issues/show.html.haml
+++ b/app/views/issues/show.html.haml
@@ -2,7 +2,7 @@
   .secondary-navigation
     %ul.wat-cf
       %li.first= link_to t("layout.issues.list"), project_issues_path(@project)
-      %li= link_to t("layout.issues.edit"), edit_project_issue_path(@project, @issue.serial_id) if can? :edit, @issue
+      %li= link_to t("layout.issues.edit"), edit_project_issue_path(@project, @issue) if can? :edit, @issue
   .content
     .inner
       %p
diff --git a/config/locales/ru.yml b/config/locales/ru.yml
index 97eea839c..892d6ff7c 100644
--- a/config/locales/ru.yml
+++ b/config/locales/ru.yml
@@ -382,10 +382,13 @@ ru:
 
     comment:
       saved: Комментарий успешно сохранен
+      save_error: Ошибка сохранения комментария
       destroyed: Комментарий удален
 
     issue:
       saved: Задача успешно сохранена
+      save_error: Не удалось сохранить задачу
+      destroyed: Задача успешно удалена
 
     project:
       saved: Проект успешно сохранен
@@ -406,10 +409,6 @@ ru:
       destroyed: Группа успешно удалена
       user_uname_exists: Пользователь с таким именем уже зарегестрирован
 
-    role:
-      saved: Роль успешно сохранена
-      save_error: Ошибка сохранения роли
-
     repository:
       saved: Репозиторий успешно добавлен
       save_error: Не удалось добавить репозиторий
@@ -427,7 +426,7 @@ ru:
 
     platform:
       saved: Платформа успешно добавлена
-      saved_error: Не удалось создать платформу
+      save_error: Не удалось создать платформу
       freezed: Платформа успешно заморожена
       freeze_error: Не удалось заморозить платформу, попробуйте еще раз
       unfreezed: Платформа успешно разморожена
diff --git a/config/routes.rb b/config/routes.rb
index 4a0227e63..dcea79076 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -78,10 +78,8 @@ Rosa::Application.routes.draw do
     resources :categories, :only => [:index, :show]
   end
 
-  match "projects/:project_id/issues/:serial_id" => 'issues#show', :serial_id => /\d+/, :as => :show_issue, :via => :get
-  match "projects/:project_id/issues/:serial_id/edit" => 'issues#edit', :serial_id => /\d+/, :as => :edit_issue, :via => :get
   resources :projects do
-    resources :issues, :except => [:show] do
+    resources :issues do
       resources :comments, :only => [:edit, :create, :update, :destroy]
     end
     resource :repo, :controller => "git/repositories", :only => [:show]
diff --git a/lib/grack/base.rb b/lib/grack/base.rb
index 1d77f2af4..e2ee2a121 100644
--- a/lib/grack/base.rb
+++ b/lib/grack/base.rb
@@ -28,7 +28,7 @@ module Grack
     end
 
     def action
-      write? ? :update : :read
+      write? ? :write : :read
     end
 
     def project
diff --git a/spec/controllers/build_lists_controller_spec.rb b/spec/controllers/build_lists_controller_spec.rb
index c05d24e50..bfedd23a2 100644
--- a/spec/controllers/build_lists_controller_spec.rb
+++ b/spec/controllers/build_lists_controller_spec.rb
@@ -94,9 +94,8 @@ describe BuildListsController do
           @build_list1 = Factory(:build_list_core)
           @build_list2 = Factory(:build_list_core, :project => Factory(:project, :visibility => 'hidden'))
           @build_list3 = Factory(:build_list_core, :project => Factory(:project, :owner => @user, :visibility => 'hidden'))
-            b = Factory(:build_list_core, :project => Factory(:project, :visibility => 'hidden'))
-            b.project.relations.create :role => 'reader', :object_id => @user.id, :object_type => 'User'
-          @build_list4 = b
+          @build_list4 = Factory(:build_list_core, :project => Factory(:project, :visibility => 'hidden'))
+          @build_list4.project.relations.create :role => 'reader', :object_id => @user.id, :object_type => 'User'
         end
 
         it 'should be able to perform index action' do
@@ -180,9 +179,8 @@ describe BuildListsController do
           @build_list1 = Factory(:build_list_core)
           @build_list2 = Factory(:build_list_core, :project => Factory(:project, :visibility => 'hidden'))
           @build_list3 = Factory(:build_list_core, :project => Factory(:project, :owner => @group, :visibility => 'hidden'))
-            b = Factory(:build_list_core, :project => Factory(:project, :visibility => 'hidden'))
-            b.project.relations.create :role => 'reader', :object_id => @group.id, :object_type => 'Group'
-          @build_list4 = b
+          @build_list4 = Factory(:build_list_core, :project => Factory(:project, :visibility => 'hidden'))
+          @build_list4.project.relations.create :role => 'reader', :object_id => @group.id, :object_type => 'Group'
         end
 
         it 'should be able to perform index action' do
diff --git a/spec/controllers/issues_controller_spec.rb b/spec/controllers/issues_controller_spec.rb
index 2d9ba9a46..37aad58cf 100644
--- a/spec/controllers/issues_controller_spec.rb
+++ b/spec/controllers/issues_controller_spec.rb
@@ -8,7 +8,7 @@ shared_examples_for 'issue user with project reader rights' do
   end
 
   it 'should be able to perform show action' do
-    get :show, :project_id => @project.id, :serial_id => @issue.serial_id
+    get :show, :project_id => @project.id, :id => @issue.serial_id
     response.should render_template(:show)
   end
 end
@@ -26,36 +26,36 @@ end
 
 shared_examples_for 'user with issue update rights' do
   it 'should be able to perform update action' do
-    put :update, {:id => @issue.id}.merge(@update_params)
-    response.should redirect_to(show_issue_path(@project, @issue.serial_id))
+    put :update, {:id => @issue.serial_id}.merge(@update_params)
+    response.should redirect_to([@project, @issue])
   end
 
   it 'should update issue title' do
-    put :update, {:id => @issue.id}.merge(@update_params)
+    put :update, {:id => @issue.serial_id}.merge(@update_params)
     @issue.reload.title.should == 'issue2'
   end
 end
 
 shared_examples_for 'user without issue update rights' do
   it 'should not be able to perform update action' do
-    put :update, {:id => @issue.id}.merge(@update_params)
+    put :update, {:id => @issue.serial_id}.merge(@update_params)
     response.should redirect_to(forbidden_path)
   end
 
   it 'should not update issue title' do
-    put :update, {:id => @issue.id}.merge(@update_params)
+    put :update, {:id => @issue.serial_id}.merge(@update_params)
     @issue.reload.title.should_not == 'issue2'
   end
 end
 
 shared_examples_for 'user without issue destroy rights' do
   it 'should not be able to perform destroy action' do
-    delete :destroy, :id => @issue.id, :project_id => @project.id
+    delete :destroy, :id => @issue.serial_id, :project_id => @project.id
     response.should redirect_to(forbidden_path)
   end
 
   it 'should not reduce issues count' do
-    lambda{ delete :destroy, :id => @issue.id, :project_id => @project.id }.should change{ Issue.count }.by(0)
+    lambda{ delete :destroy, :id => @issue.serial_id, :project_id => @project.id }.should change{ Issue.count }.by(0)
   end
 end
 
@@ -67,7 +67,7 @@ shared_examples_for 'project with issues turned off' do
   end
 
   it 'should not be able to perform show action' do
-    get :show, :project_id => @project_with_turned_off_issues.id, :serial_id => @turned_of_issue.serial_id
+    get :show, :project_id => @project_with_turned_off_issues.id, :id => @turned_of_issue.serial_id
     response.should redirect_to(forbidden_path)
   end
 end
diff --git a/spec/controllers/products_controller_spec.rb b/spec/controllers/products_controller_spec.rb
index 7b6e4f4f6..826568374 100644
--- a/spec/controllers/products_controller_spec.rb
+++ b/spec/controllers/products_controller_spec.rb
@@ -61,8 +61,7 @@ describe ProductsController do
   	before(:each) do
   		@user = Factory(:user)
   		set_session_for(@user)
-      r = @product.relations.create!(:object_type => 'User', :object_id => @user.id, :role => 'admin')
-      r = @platform.relations.create!(:object_type => 'User', :object_id => @user.id, :role => 'admin')
+      @platform.relations.create!(:object_type => 'User', :object_id => @user.id, :role => 'admin')
 		end
 
     it 'should be able to perform create action' do
diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb
index c17c09954..29e6aff09 100644
--- a/spec/controllers/projects_controller_spec.rb
+++ b/spec/controllers/projects_controller_spec.rb
@@ -28,7 +28,7 @@ describe ProjectsController do
   		set_session_for(@admin)
 		end
 
-    it_should_behave_like 'projects user with writer rights'
+    it_should_behave_like 'projects user with admin rights'
     it_should_behave_like 'projects user with reader rights'
 
     it 'should be able to perform create action' do
@@ -49,7 +49,7 @@ describe ProjectsController do
   		@project.relations.create!(:object_type => 'User', :object_id => @user.id, :role => 'admin')
 		end
 
-    it_should_behave_like 'projects user with writer rights'
+    it_should_behave_like 'projects user with admin rights'
     it_should_behave_like 'user with rights to view projects'
 
     it 'should be able to perform destroy action' do
@@ -90,7 +90,6 @@ describe ProjectsController do
   		@project.relations.create!(:object_type => 'User', :object_id => @user.id, :role => 'writer')
 		end
 
-    it_should_behave_like 'projects user with writer rights'
     it_should_behave_like 'projects user with reader rights'
   end
 
diff --git a/spec/models/cancan_spec.rb b/spec/models/cancan_spec.rb
index 927a38447..e245c19ff 100644
--- a/spec/models/cancan_spec.rb
+++ b/spec/models/cancan_spec.rb
@@ -50,19 +50,19 @@ describe CanCan do
 			guest_create
 		end
 
-		it 'should be able to read open platform' do
-			@ability.should be_able_to(:read, open_platform)
-		end
+    it 'should not be able to read open platform' do
+    	@ability.should_not be_able_to(:read, open_platform)
+    end
 
-		it 'should not be able to read hidden platform' do
-			@ability.should_not be_able_to(:read, hidden_platform)
-		end
+    it 'should not be able to read hidden platform' do
+    	@ability.should_not be_able_to(:read, hidden_platform)
+    end
 
-		it 'should be able to auto build projects' do
-			@ability.should be_able_to(:auto_build, Project)
-		end
+    it 'should be able to auto build projects' do
+    	@ability.should be_able_to(:auto_build, Project)
+    end
 
-		[:status_build, :pre_build, :post_build, :circle_build, :new_bbdt].each do |action|
+		[:publish_build, :status_build, :pre_build, :post_build, :circle_build, :new_bbdt].each do |action|
 			it "should be able to #{ action } build list" do
 				@ability.should be_able_to(action, BuildList)
 			end
@@ -78,11 +78,13 @@ describe CanCan do
       user_create
     end
 
-    [Platform, User, Repository].each do |model_name|
-      it "should not be able to create #{ model_name.to_s }" do
+    [Platform, Repository].each do |model_name|
+      it "should not be able to read #{model_name}" do
         @ability.should be_able_to(:read, model_name)
       end
     end
+    
+    it { @ability.should be_able_to(:show, User) }
 
     it "shoud be able to read another user object" do
       admin_create
@@ -225,8 +227,10 @@ describe CanCan do
           @platform.update_attribute(:owner, @user)
         end
 
-        it 'should be able to manage platform' do
-          @ability.should be_able_to(:manage, @platform)
+        [:read, :update, :destroy, :freeze, :unfreeze].each do |action|
+          it "should be able to #{action} platform" do
+            @ability.should be_able_to(action, @platform)
+          end
         end
       end
 
@@ -251,11 +255,16 @@ describe CanCan do
           @repository.update_attribute(:owner, @user)
         end
 
-        [:manage, :add_project, :remove_project, :change_visibility, :settings].each do |action|
-          it "should be able to #{ action } repository" do
+        [:read, :update, :destroy, :add_project, :remove_project, :change_visibility, :settings].each do |action|
+          it "should be able to #{action} repository" do
             @ability.should be_able_to(action, @repository)
           end
         end
+
+        it do
+          @repository.platform.update_attribute(:owner, @user)
+          @ability.should be_able_to(:create, @repository)
+        end
       end
 
       context 'with read rights' do
@@ -272,7 +281,7 @@ describe CanCan do
     context 'build list relations' do
       before(:each) do
         @project = Factory(:project)
-        @project.relations.create!(:object_id => @user.id, :object_type => 'User', :role => 'reader')
+        @project.relations.create!(:object_id => @user.id, :object_type => 'User', :role => 'writer')
         @build_list = Factory(:build_list, :project => @project)
       end
 
diff --git a/spec/support/shared_examples/projects_controller.rb b/spec/support/shared_examples/projects_controller.rb
index 37cd28b28..0dff8f8d2 100644
--- a/spec/support/shared_examples/projects_controller.rb
+++ b/spec/support/shared_examples/projects_controller.rb
@@ -7,7 +7,7 @@ shared_examples_for 'projects user with reader rights' do
   end
 end
 
-shared_examples_for 'projects user with writer rights' do
+shared_examples_for 'projects user with admin rights' do
   it 'should be able to perform update action' do
     put :update, {:id => @project.id}.merge(@update_params)
     response.should redirect_to(project_path(@project))