Merge pull request #212 from abf/rosa-build:211-display-only-platforms-for-which-user-has-access-on-read

#211: Display only platforms for which user has access on read.

app/controllers/api/v1/build_lists_controller.rb

@@ -8,7 +8,7 @@ class Api::V1::BuildListsController < Api::V1::BaseController
   load_and_authorize_resource :build_list, :only => [:show, :create, :cancel, :publish, :reject_publish, :create_container]
 
   def index
-    filter = BuildList::Filter.new(@project, current_user, params[:filter] || {})
+    filter = BuildList::Filter.new(@project, current_user, current_ability, params[:filter] || {})
     @build_lists = filter.find.scoped(:include => [:save_to_platform, :project, :user, :arch])
     @build_lists = @build_lists.recent.paginate(paginate_params)
   end

app/controllers/api/v1/platforms_controller.rb

@@ -26,7 +26,7 @@ class Api::V1::PlatformsController < Api::V1::BaseController
 
     user = User.find_by_authentication_token token
     @current_ability, @current_user = nil, user
-    if user && can?(:read, platform)
+    if user && can?(:show, platform)
       render :nothing => true
     else
       render :nothing => true, :status => 403

app/controllers/projects/build_lists_controller.rb

@@ -22,13 +22,23 @@ class Projects::BuildListsController < Projects::BaseController
 
   def index
     @action_url = @project ? search_project_build_lists_path(@project) : search_build_lists_path
-    @filter = BuildList::Filter.new(@project, current_user, params[:filter] || {})
+    @filter     = BuildList::Filter.new(@project, current_user, current_ability, params[:filter] || {})
 
-    page = params[:page].to_i == 0 ? nil : params[:page]
     @per_page = BuildList::Filter::PER_PAGE.include?(params[:per_page].to_i) ? params[:per_page].to_i : 25
-    @bls = @filter.find.recent.paginate :page => page, :per_page => @per_page
+    @bls      = @filter.find.recent.paginate(
-    @build_lists = BuildList.where(:id => @bls.pluck("#{BuildList.table_name}.id")).recent
+      :page     => (params[:page].to_i == 0 ? nil : params[:page]),
-    @build_lists = @build_lists.includes [:save_to_platform, :save_to_repository, :arch, :user, :project => [:owner]]
+      :per_page => @per_page
+    )
+    @build_lists = BuildList.where(:id => @bls.pluck(:id)).recent
+                            .includes(
+                              :save_to_platform,
+                              :save_to_repository,
+                              :build_for_platform,
+                              :arch,
+                              :user,
+                              :source_packages,
+                              :project => [:owner]
+                            )
 
     @build_server_status = AbfWorker::StatusInspector.projects_status
   end
@@ -57,7 +67,7 @@ class Projects::BuildListsController < Projects::BaseController
         @build_list.priority = current_user.build_priority # User builds more priority than mass rebuild with zero priority
 
         flash_options = {:project_version => @build_list.project_version, :arch => arch.name, :build_for_platform => build_for_platform.name}
-        if @build_list.save
+        if authorize!(:create, @build_list) && @build_list.save
           notices << t("flash.build_list.saved", flash_options)
         else
           errors << t("flash.build_list.save_error", flash_options)

app/helpers/build_lists_helper.rb

@@ -13,6 +13,21 @@ module BuildListsHelper
     end
   end
 
+  def availables_main_platforms
+    # Main platforms with repositories
+    Platform.main.accessible_by(current_ability, :show)
+            .joins(:repositories).uniq
+  end
+
+  def mass_build_options(selected_id)
+    options_from_collection_for_select(
+      MassBuild.recent.limit(15),
+      :id,
+      :name,
+      selected_id
+    )
+  end
+
   def build_list_options_for_new_core
     [
       [I18n.t("layout.true_"), 1],
@@ -98,7 +113,7 @@ module BuildListsHelper
   end
 
   def get_version_release build_list
-    pkg = build_list.packages.where(:package_type => 'source', :project_id => build_list.project_id).first
+    pkg = build_list.source_packages.first
     "#{pkg.version}-#{pkg.release}" if pkg.present?
   end
 end

app/models/ability.rb

@@ -82,7 +82,11 @@ 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, 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) &&
+          (build_list.build_for_platform.blank? || can?(:show, build_list.build_for_platform))
+        }
 
         can(:publish, BuildList) do |build_list|
           if build_list.build_published?

app/models/build_list.rb

@@ -16,6 +16,7 @@ class BuildList < ActiveRecord::Base
   belongs_to :mass_build, :counter_cache => true
   has_many :items, :class_name => "BuildList::Item", :dependent => :destroy
   has_many :packages, :class_name => "BuildList::Package", :dependent => :destroy
+  has_many :source_packages, :class_name => "BuildList::Package", :conditions => {:package_type => 'source'}
 
   UPDATE_TYPES = %w[bugfix security enhancement recommended newpackage]
   RELEASE_UPDATE_TYPES = %w[bugfix security]
@@ -34,16 +35,15 @@ class BuildList < ActiveRecord::Base
     errors.add(:build_for_platform, I18n.t('flash.build_list.wrong_build_for_platform')) unless build_for_platform.main?
   }
   validate lambda {
-    errors.add(:save_to_repository, I18n.t('flash.build_list.wrong_repository')) unless save_to_repository_id.in? save_to_platform.repositories.map(&:id)
+    errors.add(:save_to_repository, I18n.t('flash.build_list.wrong_repository')) if save_to_repository.platform_id != save_to_platform.id
   }
   validate lambda {
-    include_repos.each {|ir|
+    errors.add(:save_to_repository, I18n.t('flash.build_list.wrong_include_repos')) if build_for_platform.repositories.where(:id => include_repos).count != include_repos.size
-      errors.add(:save_to_repository, I18n.t('flash.build_list.wrong_include_repos')) unless build_for_platform.repository_ids.include? ir.to_i
-    }
   }
   validate lambda {
     errors.add(:save_to_repository, I18n.t('flash.build_list.wrong_project')) unless save_to_repository.projects.exists?(project_id)
   }
+  before_validation lambda { self.include_repos = include_repos.uniq if include_repos.present? }, :on => :create
   before_validation :prepare_extra_repositories,  :on => :create
   before_validation :prepare_extra_build_lists,   :on => :create
 
@@ -106,13 +106,13 @@ class BuildList < ActiveRecord::Base
     s = s.where(:save_to_platform_id => save_to_platform.id) if save_to_platform && save_to_platform.main?
     s
   }
-  scope :for_status, lambda {|status| where(:status => status) }
+  scope :for_status, lambda {|status| where(:status => status) if status.present? }
   scope :for_user, lambda { |user| where(:user_id => user.id)  }
   scope :for_platform, lambda { |platform| where(:build_for_platform_id => platform)  }
   scope :by_mass_build, lambda { |mass_build| where(:mass_build_id => mass_build) }
-  scope :scoped_to_arch, lambda {|arch| where(:arch_id => arch) }
+  scope :scoped_to_arch, lambda {|arch| where(:arch_id => arch) if arch.present? }
-  scope :scoped_to_save_platform, lambda {|pl_id| where(:save_to_platform_id => pl_id) }
+  scope :scoped_to_save_platform, lambda {|pl_id| where(:save_to_platform_id => pl_id) if pl_id.present? }
-  scope :scoped_to_project_version, lambda {|project_version| where(:project_version => project_version) }
+  scope :scoped_to_project_version, lambda {|project_version| where(:project_version => project_version) if project_version.present? }
   scope :scoped_to_is_circle, lambda {|is_circle| where(:is_circle => is_circle) }
   scope :for_creation_date_period, lambda{|start_date, end_date|
     s = scoped
@@ -122,11 +122,11 @@ class BuildList < ActiveRecord::Base
   }
   scope :for_notified_date_period, lambda{|start_date, end_date|
     s = scoped
-    s = s.where(["#{table_name}.updated_at >= ?", start_date]) if start_date
+    s = s.where("#{table_name}.updated_at >= ?", start_date)  if start_date.present?
-    s = s.where(["#{table_name}.updated_at <= ?", end_date]) if end_date
+    s = s.where("#{table_name}.updated_at <= ?", end_date)    if end_date.present?
     s
   }
-  scope :scoped_to_project_name, lambda {|project_name| joins(:project).where('projects.name LIKE ?', "%#{project_name}%")}
+  scope :scoped_to_project_name, lambda {|project_name| joins(:project).where('projects.name LIKE ?', "%#{project_name}%") if project_name.present? }
   scope :scoped_to_new_core, lambda {|new_core| where(:new_core => new_core)}
   scope :outdated, where("#{table_name}.created_at < ? AND #{table_name}.status <> ? OR #{table_name}.created_at < ?", Time.now - LIVE_TIME, BUILD_PUBLISHED, Time.now - MAX_LIVE_TIME)
   scope :published_container, where(:container_status => BUILD_PUBLISHED)

app/models/build_list/filter.rb

@@ -2,9 +2,8 @@
 class BuildList::Filter
   PER_PAGE = [25, 50, 100]
 
-  def initialize(project, user, options = {})
+  def initialize(project, user, current_ability, options = {})
-    @project = project
+    @project, @user, @current_ability = project, user, current_ability
-    @user = user
     set_options(options)
   end
 
@@ -14,15 +13,16 @@ class BuildList::Filter
     if @options[:id]
       build_lists = build_lists.where(:id => @options[:id])
     else
-      build_lists = build_lists.accessible_by(::Ability.new(@user), @options[:ownership].to_sym) if @options[:ownership]
       build_lists = build_lists.scoped_to_new_core(@options[:new_core] == '0' ? nil : true) if @options[:new_core].present?
-      build_lists = build_lists.for_status(@options[:status]) if @options[:status]
-      build_lists = build_lists.scoped_to_arch(@options[:arch_id]) if @options[:arch_id]
-      build_lists = build_lists.scoped_to_save_platform(@options[:platform_id]) if @options[:platform_id]
-      build_lists = build_lists.scoped_to_project_version(@options[:project_version]) if @options[:project_version]
-      build_lists = build_lists.scoped_to_project_name(@options[:project_name]) if @options[:project_name]
       build_lists = build_lists.by_mass_build(@options[:mass_build_id]) if @options[:mass_build_id]
-      build_lists = build_lists.for_notified_date_period(@options[:updated_at_start], @options[:updated_at_end]) if @options[:updated_at_start] || @options[:updated_at_end]
+      build_lists = build_lists.accessible_by(@current_ability, @options[:ownership].to_sym) if @options[:ownership]
+
+      build_lists = build_lists.for_status(@options[:status])
+                               .scoped_to_arch(@options[:arch_id])
+                               .scoped_to_save_platform(@options[:platform_id])
+                               .scoped_to_project_version(@options[:project_version])
+                               .scoped_to_project_name(@options[:project_name])
+                               .for_notified_date_period(@options[:updated_at_start], @options[:updated_at_end])
     end
 
     build_lists

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

@@ -32,13 +32,13 @@
       .column
         %h3.medium= t 'activerecord.models.platform'
         .lineForm.aside
-          = f.select :platform_id, Platform.main.collect{|pl| [pl.name, pl.id]}, {:include_blank => true, :selected => @filter.platform_id}, html_options.merge(:id => 'platform')
+          = f.select :platform_id, availables_main_platforms.collect{|pl| [pl.name, pl.id]}, {:include_blank => true, :selected => @filter.platform_id}, html_options.merge(:id => 'platform')
         %h3.medium= t 'activerecord.attributes.build_list.arch'
         .lineForm.aside
           = f.select :arch_id, Arch.recent.collect{|arch| [arch.name, arch.id]}, {:include_blank => true, :selected => @filter.arch_id}, html_options.merge(:id => 'architecture')
         %h3.medium= t 'activerecord.models.mass_build'
         .lineForm.aside
-          = f.select :mass_build_id, options_from_collection_for_select( MassBuild.recent, :id, :name, @filter.mass_build_id ), {:include_blank => true, :selected => @filter.mass_build_id},
+          = f.select :mass_build_id, mass_build_options(@filter.mass_build_id), {:include_blank => true, :selected => @filter.mass_build_id},
             html_options.merge(:id => 'mass_build')
 
       .column

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

@@ -3,8 +3,7 @@
   %section.left
     %h3= t("activerecord.attributes.build_list.build_for_platform")
     .all_platforms
-      - Platform.main.each do |pl|
+      - availables_main_platforms.each do |pl|
-        - if pl.repository_ids.size > 0
         .both
           %div{:id => "build_for_pl_#{pl.id}", :class => 'build_for_pl'}= pl.name
           .offset25= render 'include_repos', :platform => pl

spec/controllers/api/v1/build_lists_controller_spec.rb

@@ -116,7 +116,7 @@ describe Api::V1::BuildListsController do
 
         # Create and show params:
         @create_params = {:build_list => @build_list.attributes.symbolize_keys.merge(:qwerty=>'!')} # wrong parameter
-        @create_params = @create_params.merge(:arches => [@params[:arch_id]], :build_for_platforms => [@params[:build_for_platform_id]], :format => :json)
+        @create_params = @create_params.merge(:arches => [@params[:arch_id]], :build_for_platform_id => @platform.id, :format => :json)
         any_instance_of(Project, :versions => ['v1.0', 'v2.0'])
 
         http_login(@user)
@@ -466,10 +466,25 @@ describe Api::V1::BuildListsController do
         context 'if user is project owner' do
           before(:each) {http_login(@owner_user)}
           it_should_behave_like 'create build list via api'
+
+          context 'no ability to read build_for_platform' do
+            before do
+              repository = FactoryGirl.create(:repository)
+              repository.platform.change_visibility
+              Platform.where(:id => @platform.id).update_all(:platform_type => 'personal')
+              @create_params[:build_list].merge!({
+                :include_repos          => [repository.id],
+                :build_for_platform_id  => repository.platform_id
+              })
+            end
+            it_should_behave_like 'not create build list via api'
+          end
+
         end
 
         context 'if user is project read member' do
           before(:each) {http_login(@member_user)}
+          it_should_behave_like 'not create build list via api'
         end
       end
 
@@ -510,7 +525,7 @@ describe Api::V1::BuildListsController do
 
         # Create and show params:
         @create_params = {:build_list => @build_list.attributes.symbolize_keys}
-        @create_params = @create_params.merge(:arches => [@params[:arch_id]], :build_for_platforms => [@params[:build_for_platform_id]], :format => :json)
+        @create_params = @create_params.merge(:arches => [@params[:arch_id]], :build_for_platform_id => @platform.id, :format => :json)
         any_instance_of(Project, :versions => ['v1.0', 'v2.0'])
 
         # Groups:

spec/controllers/projects/build_lists_controller_spec.rb

@@ -62,7 +62,7 @@ describe Projects::BuildListsController do
     end
   end
 
-  shared_examples_for 'not create build list' do
+  shared_examples_for 'not create build list' do |skip_new = false|
     before {
       @project.update_attribute(:repositories, @platform.repositories)
     }
@@ -70,7 +70,7 @@ describe Projects::BuildListsController do
     it 'should not be able to perform new action' do
       get :new, :owner_name => @project.owner.uname, :project_name => @project.name
       response.should redirect_to(forbidden_url)
-    end
+    end unless skip_new
 
     it 'should not be able to perform create action' do
       post :create, {:owner_name => @project.owner.uname, :project_name => @project.name}.merge(@create_params)
@@ -259,6 +259,17 @@ describe Projects::BuildListsController do
           it_should_behave_like 'show build list'
           it_should_behave_like 'create build list'
 
+          context 'no ability to read build_for_platform' do
+            before do
+              repository = FactoryGirl.create(:repository)
+              repository.platform.change_visibility
+              Platform.where(:id => @platform.id).update_all(:platform_type => 'personal')
+              @create_params[:build_list].merge!({:include_repos => [repository.id]})
+              @create_params[:build_for_platforms] = [repository.platform_id]
+            end
+            it_should_behave_like 'not create build list', true
+          end
+
         end
 
         context 'if user is project read member' do