Merge pull request #150 from abf/rosa-build:135-reject_build_lists

[refs #135] extend ability to reject publish build lists

app/assets/stylesheets/design/custom.scss

@@ -2035,3 +2035,7 @@ article .activity .top {
   -moz-box-shadow: 0;
   box-shadow: 0;
 }
+
+a.button.reject_publish, a.button.create_container {
+  height: 15px;
+}

app/controllers/api/v1/build_lists_controller.rb

@@ -34,11 +34,11 @@ class Api::V1::BuildListsController < Api::V1::BaseController
 
   def publish
     @build_list.publisher = current_user
-    @build_list.save
     render_json :publish
   end
 
   def reject_publish
+    @build_list.publisher = current_user
     render_json :reject_publish
   end
 

app/controllers/projects/build_lists_controller.rb

@@ -79,14 +79,43 @@ class Projects::BuildListsController < Projects::BaseController
     @item_groups = @build_list.items.group_by_level
   end
 
-  def update
-    if params[:publish].present? and can?(:publish, @build_list)
-      publish
-    elsif params[:reject_publish].present? and can?(:reject_publish, @build_list)
-      reject_publish
+  def publish
+    @build_list.update_type = params[:build_list][:update_type] if params[:build_list][:update_type].present?
+
+    if params[:attach_advisory].present? and params[:attach_advisory] != 'no' and !@build_list.advisory
+
+      unless @build_list.update_type.in? BuildList::RELEASE_UPDATE_TYPES
+        redirect_to :back, :notice => t('layout.build_lists.publish_fail') and return
+      end
+
+      if params[:attach_advisory] == 'new'
+        # create new advisory
+        unless @build_list.associate_and_create_advisory(params[:build_list][:advisory])
+          redirect_to :back, :notice => t('layout.build_lists.publish_fail') and return
+        end
       else
-      # King Arthur, we are under attack!
-      redirect_to :forbidden and return
+        # attach existing advisory
+        a = Advisory.where(:advisory_id => params[:attach_advisory]).first
+        unless (a && a.attach_build_list(@build_list))
+          redirect_to :back, :notice => t('layout.build_lists.publish_fail') and return
+        end
+      end
+    end
+
+    @build_list.publisher = current_user
+    if @build_list.publish
+      redirect_to :back, :notice => t('layout.build_lists.publish_success')
+    else
+      redirect_to :back, :notice => t('layout.build_lists.publish_fail')
+    end
+  end
+
+  def reject_publish
+    @build_list.publisher = current_user
+    if @build_list.reject_publish
+      redirect_to :back, :notice => t('layout.build_lists.reject_publish_success')
+    else
+      redirect_to :back, :notice => t('layout.build_lists.reject_publish_fail')
     end
   end
 
@@ -118,44 +147,4 @@ class Projects::BuildListsController < Projects::BaseController
   def find_build_list
     @build_list = BuildList.find(params[:id])
   end
-
-  def publish
-    @build_list.update_type = params[:build_list][:update_type] if params[:build_list][:update_type].present?
-
-    if params[:attach_advisory].present? and params[:attach_advisory] != 'no' and !@build_list.advisory
-
-      unless @build_list.update_type.in? BuildList::RELEASE_UPDATE_TYPES
-        redirect_to :back, :notice => t('layout.build_lists.publish_fail') and return
-      end
-
-      if params[:attach_advisory] == 'new'
-        # create new advisory
-        unless @build_list.associate_and_create_advisory(params[:build_list][:advisory])
-          redirect_to :back, :notice => t('layout.build_lists.publish_fail') and return
-        end
-      else
-        # attach existing advisory
-        a = Advisory.where(:advisory_id => params[:attach_advisory]).first
-        unless (a && a.attach_build_list(@build_list))
-          redirect_to :back, :notice => t('layout.build_lists.publish_fail') and return
-        end
-      end
-
-    end
-
-    @build_list.publisher = current_user
-    if @build_list.save && @build_list.can_publish? && @build_list.now_publish
-      redirect_to :back, :notice => t('layout.build_lists.publish_success')
-    else
-      redirect_to :back, :notice => t('layout.build_lists.publish_fail')
-    end
-  end
-
-  def reject_publish
-    if @build_list.reject_publish
-      redirect_to :back, :notice => t('layout.build_lists.reject_publish_success')
-    else
-      redirect_to :back, :notice => t('layout.build_lists.reject_publish_fail')
-    end
-  end
 end

app/models/ability.rb

@@ -82,7 +82,7 @@ class Ability
         can [:read, :log, :related, :everything], BuildList, :project => {:owner_type => 'User', :owner_id => user.id}
         can [:read, :log, :related, :everything], BuildList, :project => {:owner_type => 'Group', :owner_id => user.group_ids}
         can([:read, :log, :everything], BuildList, read_relations_for('build_lists', 'projects')) {|build_list| can? :read, build_list.project}
-        can([:create, :update], BuildList) {|build_list| build_list.project.is_package && can?(:write, build_list.project)}
+        can(:create, BuildList) {|build_list| build_list.project.is_package && can?(:write, build_list.project)}
 
         can(:publish, BuildList) do |build_list|
           if build_list.build_published?
@@ -92,9 +92,13 @@ class Ability
               can?(:write, build_list.project) : local_admin?(build_list.save_to_platform)
           end
         end
-        can([:reject_publish, :create_container], BuildList) do |build_list|
+        can(:create_container, BuildList) do |build_list|
           local_admin?(build_list.save_to_platform)
         end
+        can(:reject_publish, BuildList) do |build_list|
+          build_list.save_to_repository.publish_without_qa ?
+              can?(:write, build_list.project) : local_admin?(build_list.save_to_platform)
+        end
         can([:cancel, :create_container], BuildList) {|build_list| can?(:write, build_list.project)}
 
         can [:read, :owned, :related, :members], Platform, :owner_type => 'User', :owner_id => user.id

app/models/build_list.rb

@@ -198,7 +198,7 @@ class BuildList < ActiveRecord::Base
     end
 
     event :reject_publish do
-      transition [:success, :failed_publish, :tests_failed] => :rejected_publish, :if => :can_reject_publish?
+      transition [:success, :failed_publish, :tests_failed] => :rejected_publish
     end
 
     event :build_success do
@@ -289,10 +289,6 @@ class BuildList < ActiveRecord::Base
     BuildList.where(:id => extra_build_lists).where('status != ?', BUILD_PUBLISHED).count == 0
   end
 
-  def can_reject_publish?
-    [SUCCESS, FAILED_PUBLISH, TESTS_FAILED].include?(status) && !save_to_repository.publish_without_qa
-  end
-
   def add_to_queue
     add_job_to_abf_worker_queue
     update_column(:bs_id, id)

app/views/projects/build_lists/show.html.haml

@@ -158,10 +158,11 @@
       - confirm = @build_list.tests_failed? ? t('layout.build_lists.tests_failed') : t('layout.confirm')
       = submit_tag t("layout.publish"), :confirm => confirm, :name => 'publish'
   - if @build_list.can_reject_publish? && can?(:reject_publish, @build_list)
-      = submit_tag t("layout.reject_publish"), :confirm => t("layout.confirm"), :name => 'reject_publish'
+    = link_to t('layout.reject_publish'), reject_publish_build_list_path(@build_list),
+              :method => :put, :confirm => t("layout.confirm"), :class => 'button reject_publish'
   - if @build_list.can_create_container? && can?(:create_container, @build_list)
     = link_to t("layout.build_lists.create_container"), create_container_build_list_path(@build_list),
-              :method => :put, :confirm => t("layout.confirm"), :class => 'button' 
+              :method => :put, :confirm => t("layout.confirm"), :class => 'button create_container'
 
 .hr
 %h3= t("layout.build_lists.items_header")

config/routes.rb

@@ -232,11 +232,13 @@ Rosa::Application.routes.draw do
   end
 
   scope :module => 'projects' do
-    resources :build_lists, :only => [:index, :show, :update] do
+    resources :build_lists, :only => [:index, :show] do
       member do
         put :cancel
         put :create_container
         get :log
+        put :publish
+        put :reject_publish
       end
       collection {
         post :search

spec/controllers/api/v1/build_lists_controller_spec.rb

@@ -416,6 +416,49 @@ describe Api::V1::BuildListsController do
             @build_list.reload.status.should == BuildList::SUCCESS
           end
         end
+
+        context 'if user is project reader' do
+          before(:each) do
+            @another_user = FactoryGirl.create(:user)
+            @build_list.update_column(:status, BuildList::SUCCESS)
+            @build_list.save_to_repository.update_column(:publish_without_qa, true)
+            @build_list.project.collaborators.create(:actor_type => 'User', :actor_id => @another_user.id, :role => 'reader')
+            http_login(@another_user)
+            do_reject_publish
+          end
+
+          it "should return access violation message" do
+            response.body.should == {"message" => "Access violation to this page!"}.to_json
+          end
+
+          it "should not change status of build list" do
+            do_reject_publish
+            @build_list.reload.status.should == BuildList::SUCCESS
+          end
+        end
+
+        context 'if user is project writer' do
+          before(:each) do
+            @another_user = FactoryGirl.create(:user)
+            @build_list.update_column(:status, BuildList::SUCCESS)
+            @build_list.save_to_repository.update_column(:publish_without_qa, true)
+            @build_list.project.relations.create!(:actor_type => 'User', :actor_id => @another_user.id, :role => 'writer')
+            http_login(@another_user)
+            do_reject_publish
+          end
+
+          it "should return correct json message" do
+            response.body.should == { :build_list => {:id => @build_list.id, :message => I18n.t('layout.build_lists.reject_publish_success')} }.to_json
+          end
+
+          it 'should return 200 response code' do
+            response.should be_success
+          end
+
+          it "should reject publish build list" do
+            @build_list.reload.status.should == BuildList::REJECTED_PUBLISH
+          end
+        end
       end
 
       context 'for open project' do
@@ -745,6 +788,5 @@ describe Api::V1::BuildListsController do
         end
       end
     end
-
   end
 end

spec/controllers/api/v1/platforms_controller_spec.rb

@@ -225,13 +225,17 @@ describe Api::V1::PlatformsController do
       response.status.should == 401
     end
 
-    [:show, :platforms_for_build].each do |action|
-      it "should not be able to perform #{ action } action", :anonymous_access  => false do
-        get action, :format => :json
+    it "should not be able to perform platforms_for_build action", :anonymous_access  => false do
+      get :platforms_for_build, :format => :json
       response.status.should == 401
     end
+
+    it "should not be able to perform show action", :anonymous_access  => false do
+      get :show, :id => @platform, :format => :json
+      response.status.should == 401
     end
 
+
     it 'should be able to perform members action', :anonymous_access  => true do
       get :members, :id => @platform.id, :format => :json
       response.should render_template(:members)

spec/controllers/projects/build_lists_controller_spec.rb

@@ -76,7 +76,6 @@ describe Projects::BuildListsController do
       post :create, {:owner_name => @project.owner.uname, :project_name => @project.name}.merge(@create_params)
       response.should redirect_to(forbidden_url)
     end
-
   end
 
   before { stub_symlink_methods }
@@ -113,6 +112,7 @@ describe Projects::BuildListsController do
 
     context 'for user' do
       before(:each) do
+        any_instance_of(BuildList, :current_duration => 100)
         @build_list = FactoryGirl.create(:build_list_core)
         @project = @build_list.project
         @owner_user = @project.owner
@@ -123,6 +123,102 @@ describe Projects::BuildListsController do
         @user = FactoryGirl.create(:user)
         set_session_for(@user)
         @show_params = {:owner_name => @project.owner.uname, :project_name => @project.name, :id => @build_list.id}
+        @build_list.save_to_repository.update_column(:publish_without_qa, false)
+        @request.env['HTTP_REFERER'] = build_list_path(@build_list)
+      end
+
+      context "do reject_publish" do
+        before(:each) {@build_list.save_to_repository.update_column(:publish_without_qa, true)}
+
+        def do_reject_publish
+          put :reject_publish, :id => @build_list
+        end
+
+        context 'if user is project owner' do
+          before(:each) do
+            set_session_for(@owner_user)
+            @build_list.update_column(:status, BuildList::SUCCESS)
+            @build_list.save_to_platform.update_column(:released, true)
+            do_reject_publish
+          end
+
+          context "if it has :success status" do
+            it 'should return 302 response code' do
+              response.status.should == 302
+            end
+
+            it "should reject publish build list" do
+              @build_list.reload.status.should == BuildList::REJECTED_PUBLISH
+            end
+          end
+
+          context "if it has another status" do
+            before(:each) do
+              @build_list.update_column(:status, BuildList::BUILD_ERROR)
+              do_reject_publish
+            end
+
+            it "should not change status of build list" do
+              @build_list.reload.status.should == BuildList::BUILD_ERROR
+            end
+          end
+        end
+
+        context 'if user is not project owner' do
+          before(:each) do
+            @build_list.update_column(:status, BuildList::SUCCESS)
+            @build_list.save_to_platform.update_column(:released, true)
+            do_reject_publish
+          end
+
+          it "should redirect to forbidden page" do
+            response.should redirect_to(forbidden_url)
+          end
+
+          it "should not change status of build list" do
+            do_reject_publish
+            @build_list.reload.status.should == BuildList::SUCCESS
+          end
+        end
+
+        context 'if user is project reader' do
+          before(:each) do
+            @another_user = FactoryGirl.create(:user)
+            @build_list.update_column(:status, BuildList::SUCCESS)
+            @build_list.save_to_repository.update_column(:publish_without_qa, true)
+            @build_list.project.collaborators.create(:actor_type => 'User', :actor_id => @another_user.id, :role => 'reader')
+            set_session_for(@another_user)
+            do_reject_publish
+          end
+
+          it "should redirect to forbidden page" do
+            response.should redirect_to(forbidden_url)
+          end
+
+          it "should not change status of build list" do
+            do_reject_publish
+            @build_list.reload.status.should == BuildList::SUCCESS
+          end
+        end
+
+        context 'if user is project writer' do
+          before(:each) do
+            @writer_user = FactoryGirl.create(:user)
+            @build_list.update_column(:status, BuildList::SUCCESS)
+            @build_list.save_to_repository.update_column(:publish_without_qa, true)
+            @build_list.project.relations.create!(:actor_type => 'User', :actor_id => @writer_user.id, :role => 'writer')
+            set_session_for(@writer_user)
+            do_reject_publish
+          end
+
+          it 'should return 302 response code' do
+            response.status.should == 302
+          end
+
+          it "should reject publish build list" do
+            @build_list.reload.status.should == BuildList::REJECTED_PUBLISH
+          end
+        end
       end
 
       context 'for all build lists' do

spec/controllers/projects/issues_controller_spec.rb

@@ -129,12 +129,12 @@ end
 
 shared_examples_for 'project with issues turned off' do
   it 'should not be able to perform index action' do
-    get :index, :project_id => @project_with_turned_off_issues.id
+    get :index, :owner_name => @project_with_turned_off_issues.owner.uname, :project_name => @project_with_turned_off_issues.name
     response.should redirect_to(forbidden_path)
   end
 
   it 'should not be able to perform show action' do
-    get :show, :project_id => @project_with_turned_off_issues.id, :id => @turned_of_issue.serial_id
+    get :show, :owner_name => @project_with_turned_off_issues.owner.uname, :project_name => @project_with_turned_off_issues.name, :id => @turned_of_issue.serial_id
     response.should redirect_to(forbidden_path)
   end
 end

spec/controllers/projects/pull_requests_controller_spec.rb

@@ -159,7 +159,7 @@ end
 shared_examples_for 'pull request when project with issues turned off' do
   before { @project.update_attributes(:has_issues => false) }
   it 'should be able to perform index action' do
-    get :index, :project_id => @project.id
+    get :index, :owner_name => @project.owner.uname, :project_name => @project.name
     response.should render_template(:index)
   end