#465: updated Api::V1::AdvisoriesController, specs

app/controllers/api/v1/advisories_controller.rb

@@ -1,27 +1,25 @@
 class Api::V1::AdvisoriesController < Api::V1::BaseController
   before_action :authenticate_user!
   skip_before_action :authenticate_user!, only: [:index, :show] if APP_CONFIG['anonymous_access']
-  load_resource :advisory, find_by: :advisory_id
-  before_action :find_and_authorize_build_list, only: [:create, :update]
-  authorize_resource :build_list, only: [:create, :update]
+  before_action :load_advisory
+  before_action :load_build_list, only: [:create, :update]
 
   def index
-    @advisories = @advisories.includes(:platforms, :projects).paginate(paginate_params)
-    respond_to :json
+    authorize :advisory
+    @advisories = Advisory.includes(:platforms, :projects).paginate(paginate_params)
   end
 
   def show
     @packages_info = @advisory.fetch_packages_info
-    respond_to :json
   end
 
   def create
     if @build_list.can_attach_to_advisory? &&
         @build_list.associate_and_create_advisory(params[:advisory]) &&
         @build_list.save
-      render_json_response @advisory, 'Advisory has been created successfully'
+      render_json_response @build_list.advisory, 'Advisory has been created successfully'
     else
-      render_validation_error @advisory, error_message(@build_list, 'Advisory has not been created')
+      render_validation_error @build_list.advisory, error_message(@build_list, 'Advisory has not been created')
     end
   end
 
@@ -36,9 +34,14 @@ class Api::V1::AdvisoriesController < Api::V1::BaseController
 
   protected
 
-  def find_and_authorize_build_list
+  def load_build_list
     @build_list = BuildList.find params[:build_list_id]
-    authorize! :local_admin_manage, @build_list.save_to_platform
+    authorize @build_list.save_to_platform, :local_admin_manage?
+  end
+
+  def load_advisory
+    @advisory = Advisory.find_by(advisory_id: params[:id]) if params[:id]
+    authorize @advisory if @advisory
   end
 
 end

app/controllers/api/v1/base_controller.rb

@@ -100,7 +100,7 @@ class Api::V1::BaseController < ApplicationController
         id: id,
         message: message
       }
-    }.to_json, status: status
+    }, status: status
   end
 
   def render_validation_error(subject, message)

app/controllers/projects/projects_controller.rb

@@ -95,10 +95,10 @@ class Projects::ProjectsController < Projects::BaseController
   end
 
   def schedule
-    authorize @project, :update?
+    authorize @project
     p_to_r = @project.project_to_repositories.where(repository_id: params[:repository_id]).first
     unless p_to_r.repository.publish_without_qa
-      authorize p_to_r.repository.platform, :update?
+      authorize p_to_r.repository.platform, :local_admin_manage?
     end
     p_to_r.user_id      = current_user.id
     p_to_r.enabled      = params[:enabled].present?
@@ -154,7 +154,7 @@ class Projects::ProjectsController < Projects::BaseController
   end
 
   def remove_user
-    authorize @project, :update?
+    authorize @project
     @project.relations.by_actor(current_user).destroy_all
     respond_to do |format|
       format.html do
@@ -166,7 +166,7 @@ class Projects::ProjectsController < Projects::BaseController
   end
 
   def autocomplete_maintainers
-    authorize @project, :update?
+    authorize @project
     term, limit = params[:query], params[:limit] || 10
     items = User.member_of_project(@project)
                 .where("users.name ILIKE ? OR users.uname ILIKE ?", "%#{term}%", "%#{term}%")
@@ -183,7 +183,7 @@ class Projects::ProjectsController < Projects::BaseController
   end
 
   def refs_list
-    authorize @project, :show?
+    authorize @project
     refs = @project.repo.branches_and_tags.map(&:name)
     @selected   = params[:selected] if refs.include?(params[:selected])
     @selected ||= @project.resolve_default_branch

app/policies/advisory_policy.rb

@@ -6,4 +6,8 @@ class AdvisoryPolicy < ApplicationPolicy
   alias_method :search?, :index?
   alias_method :show?, :index?
 
+  def update?
+    true
+  end
+
 end

app/views/platforms/base/_submenu.html.slim

@@ -27,10 +27,10 @@
               = link_to t("layout.platforms.maintainers"), platform_maintainers_path(@platform)
             li class=('active' if contr == :mass_builds)
               = link_to t("layout.platforms.mass_build"), platform_mass_builds_path(@platform)
-          - if policy(@platform.products.build).index?
+          - if policy(@platform.products.build).show?
             li class=('active' if contr == :products)
               = link_to t("layout.products.list_header"), platform_products_path(@platform)
-          - if policy(@platform.advisories.build).index?
+          - if policy(@platform.advisories.build).show?
             li class=('active' if contr == :platforms && act == :advisories)
               = link_to t("layout.advisories.list_header"), advisories_platform_path(@platform)
           - if policy(@platform).update?
@@ -39,6 +39,7 @@
           - if policy(@platform).local_admin_manage?
             li class=('active' if act == :members && contr == :platforms)
               = link_to t("layout.platforms.members"), members_platform_path(@platform)
+          - if policy(@platform).edit?
             li class=('active' if contr == :key_pairs)
               = link_to t("layout.key_pairs.header"), platform_key_pairs_path(@platform)
             li class=('active' if contr == :tokens)

app/views/projects/base/_settings_menu.html.slim

@@ -9,5 +9,6 @@ ul.nav.nav-tabs.nav-justified.boffset10[ role = 'tablist' ]
       = link_to t("layout.projects.sections"), sections_project_path(@project)
     li[ class = "#{(contr == :hooks) ? 'active' : ''}" ]
       = link_to t("layout.projects.hooks"), project_hooks_path(@project)
+  - if policy(@project).manage_collaborators?
     li[ class = "#{(act == :index && contr == :collaborators) ? 'active' : ''}" ]
       = link_to t("layout.projects.edit_collaborators"), project_collaborators_path(@project)

spec/controllers/api/v1/advisories_controller_spec.rb

@@ -3,12 +3,12 @@ require 'spec_helper'
 shared_examples_for 'api advisories user with show rights' do
   it 'should be able to perform show action' do
     get :show, id: @advisory.advisory_id, format: :json
-    response.should be_success
+    expect(response).to be_success
   end
 
   it 'should be able to perform index action' do
     get :index, format: :json
-    response.should be_success
+    expect(response).to be_success
   end
 end
 
@@ -17,15 +17,14 @@ shared_examples_for 'api advisories user with admin rights' do
     let(:params) {{ build_list_id: @build_list.id, advisory: { description: 'test' }, format: :json }}
     it 'should be able to perform create action' do
       post :create, params
-      response.should be_success
+      expect(response).to be_success
     end
     it 'ensures that advisory has been created' do
-      lambda { post :create, params }.should change{ Advisory.count }.by(1)
+      expect { post :create, params }.to change(Advisory, :count).by(1)
     end
     it 'ensures that build_list has been associated with advisory' do
       post :create, params
-      @build_list.reload
-      @build_list.advisory.should_not be_nil
+      expect(@build_list.reload.advisory).to_not be_nil
     end
   end
 
@@ -33,15 +32,14 @@ shared_examples_for 'api advisories user with admin rights' do
     let(:params) {{ id: @advisory.advisory_id, build_list_id: @build_list.id, format: :json }}
     it 'should be able to perform update action' do
       put :update, params
-      response.should be_success
+      expect(response).to be_success
     end
     it 'ensures that advisory has not been created' do
-      lambda { put :update, params }.should_not change{ Advisory.count }
+      expect { put :update, params }.to_not change(Advisory, :count)
     end
     it 'ensures that build_list has been associated with advisory' do
       put :update, params
-      @build_list.reload
-      @build_list.advisory.should_not be_nil
+      expect(@build_list.reload.advisory).to_not be_nil
     end
   end
 end
@@ -51,15 +49,14 @@ shared_examples_for 'api advisories user without admin rights' do
     let(:params) {{ build_list_id: @build_list.id, advisory: { description: 'test' }, format: :json }}
     it 'should not be able to perform create action' do
       post :create, params
-      response.should_not be_success
+      expect(response).to_not be_success
     end
     it 'ensures that advisory has not been created' do
-      lambda { post :create, params }.should_not change{ Advisory.count }
+      expect { post :create, params }.to_not change(Advisory, :count)
     end
     it 'ensures that build_list has not been associated with advisory' do
       post :create, params
-      @build_list.reload
-      @build_list.advisory.should be_nil
+      expect(@build_list.reload.advisory).to be_nil
     end
   end
 
@@ -67,15 +64,14 @@ shared_examples_for 'api advisories user without admin rights' do
     let(:params) {{ id: @advisory.advisory_id, build_list_id: @build_list.id, format: :json }}
     it 'should not be able to perform update action' do
       put :update, params
-      response.should_not be_success
+      expect(response).to_not be_success
     end
     it 'ensures that advisory has not been created' do
-      lambda { put :update, params }.should_not change{ Advisory.count }
+      expect { put :update, params }.to_not change(Advisory, :count)
     end
     it 'ensures that build_list has not been associated with advisory' do
       put :update, params
-      @build_list.reload
-      @build_list.advisory.should be_nil
+      expect(@build_list.reload.advisory).to be_nil
     end
   end
 end
@@ -86,10 +82,9 @@ describe Api::V1::AdvisoriesController, type: :controller do
     stub_symlink_methods
 
     @advisory = FactoryGirl.create(:advisory)
-    @build_list = FactoryGirl.create(:build_list)
+    @build_list = FactoryGirl.create(:build_list, status: BuildList::BUILD_PUBLISHED)
     @build_list.save_to_platform.update_column(:released, true)
     @build_list.save_to_repository.update_column(:publish_without_qa, false)
-    @build_list.update_column(:status, BuildList::BUILD_PUBLISHED)
   end
 
   context 'for guest' do
@@ -100,12 +95,12 @@ describe Api::V1::AdvisoriesController, type: :controller do
 
     it 'should not be able to perform show action', :anonymous_access  => false do
       get :show, id: @advisory.advisory_id, format: :json
-      response.should_not be_success
+      expect(response).to_not be_success
     end
 
     it 'should not be able to perform index action', :anonymous_access  => false do
       get :index, format: :json
-      response.should_not be_success
+      expect(response).to_not be_success
     end
     it_should_behave_like 'api advisories user without admin rights'
   end
@@ -119,16 +114,6 @@ describe Api::V1::AdvisoriesController, type: :controller do
     it_should_behave_like 'api advisories user without admin rights'
   end
 
-  context 'for admin' do
-    before do
-      @admin = FactoryGirl.create(:admin)
-      http_login(@admin)
-    end
-
-    it_should_behave_like 'api advisories user with show rights'
-    it_should_behave_like 'api advisories user with admin rights'
-  end
-
   context 'for user who has access to update build_list' do
     before do
       @user = FactoryGirl.create(:user)