From ec3f6e6fd317e15b7618f21e418b5bb6b4d9516d Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Tue, 25 Nov 2014 21:42:02 +0500 Subject: [PATCH 1/4] [#247] update validations Conflicts: app/models/product_build_list.rb --- app/models/authentication.rb | 2 +- app/models/build_list.rb | 8 ++++---- app/models/build_list/package.rb | 2 +- app/models/build_script.rb | 4 ++-- app/models/comment.rb | 2 +- app/models/hook.rb | 2 +- app/models/issue.rb | 2 +- app/models/key_pair.rb | 2 +- app/models/platform_arch_setting.rb | 2 +- app/models/product.rb | 2 +- app/models/product_build_list.rb | 6 +++--- app/models/project.rb | 4 ++-- app/models/project_import.rb | 2 +- app/models/project_statistic.rb | 2 +- app/models/project_tag.rb | 2 +- app/models/repository_status.rb | 2 +- app/models/settings_notifier.rb | 4 ++-- 17 files changed, 25 insertions(+), 25 deletions(-) diff --git a/app/models/authentication.rb b/app/models/authentication.rb index 68518b6e4..29a9f1bed 100644 --- a/app/models/authentication.rb +++ b/app/models/authentication.rb @@ -1,6 +1,6 @@ class Authentication < ActiveRecord::Base belongs_to :user - validates :provider, :uid, :user_id, presence: true + validates :provider, :uid, :user, presence: true validates :uid, uniqueness: { scope: :provider, case_sensitive: false } end diff --git a/app/models/build_list.rb b/app/models/build_list.rb index d608be2ad..fa7b6e35f 100644 --- a/app/models/build_list.rb +++ b/app/models/build_list.rb @@ -38,13 +38,13 @@ class BuildList < ActiveRecord::Base AUTO_PUBLISH_STATUS_TESTING = 'testing' ] - validates :project_id, + validates :project, :project_version, :arch, :include_repos, - :build_for_platform_id, - :save_to_platform_id, - :save_to_repository_id, + :build_for_platform, + :save_to_platform, + :save_to_repository, presence: true validates_numericality_of :priority, greater_than_or_equal_to: 0 validates :external_nodes, inclusion: { in: EXTERNAL_NODES }, allow_blank: true diff --git a/app/models/build_list/package.rb b/app/models/build_list/package.rb index a60dc30aa..873c1227b 100644 --- a/app/models/build_list/package.rb +++ b/app/models/build_list/package.rb @@ -9,7 +9,7 @@ class BuildList::Package < ActiveRecord::Base attr_accessible :fullname, :name, :release, :version, :sha1, :epoch, :dependent_packages - validates :build_list_id, :project_id, :platform_id, :fullname, + validates :build_list, :project, :platform, :fullname, :package_type, :name, :release, :version, presence: true validates :package_type, inclusion: PACKAGE_TYPES diff --git a/app/models/build_script.rb b/app/models/build_script.rb index 44e0c43c3..30ebbb5b2 100644 --- a/app/models/build_script.rb +++ b/app/models/build_script.rb @@ -9,8 +9,8 @@ class BuildScript < ActiveRecord::Base belongs_to :project - validates :treeish, presence: true - validates :project_id, presence: true, uniqueness: { scope: :treeish } + validates :treeish, presence: true + validates :project, presence: true, uniqueness: { scope: :treeish } scope :by_active, -> { where(status: ACTIVE) } scope :by_treeish, -> treeish { where(treeish: treeish) } diff --git a/app/models/comment.rb b/app/models/comment.rb index 5ea3c85c6..426987cb1 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -12,7 +12,7 @@ class Comment < ActiveRecord::Base belongs_to :project serialize :data - validates :body, :user_id, :commentable_id, :commentable_type, :project_id, presence: true + validates :body, :user, :commentable_id, :commentable_type, :project_id, presence: true scope :for_commit, ->(c) { where(commentable_id: c.id.hex, commentable_type: c.class) } default_scope { order(:created_at) } diff --git a/app/models/hook.rb b/app/models/hook.rb index 1692e24a0..8d251698f 100644 --- a/app/models/hook.rb +++ b/app/models/hook.rb @@ -6,7 +6,7 @@ class Hook < ActiveRecord::Base belongs_to :project before_validation :cleanup_data - validates :project_id, :data, presence: true + validates :project, :data, presence: true validates :name, presence: true, inclusion: {in: NAMES} attr_accessible :data, :name diff --git a/app/models/issue.rb b/app/models/issue.rb index 364d6a5fd..7ea86a7b8 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -35,7 +35,7 @@ class Issue < ActiveRecord::Base has_one :pull_request#, dependent: :destroy - validates :title, :body, :project_id, presence: true + validates :title, :body, :project, presence: true after_create :set_serial_id after_create :subscribe_users diff --git a/app/models/key_pair.rb b/app/models/key_pair.rb index b0fd168f9..ce87f98ac 100644 --- a/app/models/key_pair.rb +++ b/app/models/key_pair.rb @@ -7,7 +7,7 @@ class KeyPair < ActiveRecord::Base attr_accessible :public, :secret, :repository_id attr_encrypted :secret, key: APP_CONFIG['keys']['key_pair_secret_key'] - validates :repository_id, :user_id, presence: true + validates :repository, :user, presence: true validates :secret, :public, presence: true, length: { maximum: 10000 }, on: :create validates :repository_id, uniqueness: { message: I18n.t("activerecord.errors.key_pair.repo_key_exists") } diff --git a/app/models/platform_arch_setting.rb b/app/models/platform_arch_setting.rb index 381d0f8e5..b8abafecb 100644 --- a/app/models/platform_arch_setting.rb +++ b/app/models/platform_arch_setting.rb @@ -7,7 +7,7 @@ class PlatformArchSetting < ActiveRecord::Base belongs_to :arch belongs_to :platform - validates :arch_id, :platform_id, presence: true + validates :arch, :platform, presence: true validates :platform_id, uniqueness: { scope: :arch_id } validate lambda { errors.add(:platform, I18n.t('flash.platform_arch_settings.wrong_platform')) unless platform.main? diff --git a/app/models/product.rb b/app/models/product.rb index dd239b818..d8d5da9c8 100644 --- a/app/models/product.rb +++ b/app/models/product.rb @@ -8,7 +8,7 @@ class Product < ActiveRecord::Base has_many :product_build_lists, dependent: :destroy validates :name, presence: true, uniqueness: { scope: :platform_id } - validates :project_id, presence: true + validates :project, presence: true validates :main_script, :params, length: { maximum: 255 } scope :recent, -> { order(:name) } diff --git a/app/models/product_build_list.rb b/app/models/product_build_list.rb index 5c8464928..0c1d18a3f 100644 --- a/app/models/product_build_list.rb +++ b/app/models/product_build_list.rb @@ -45,11 +45,11 @@ class ProductBuildList < ActiveRecord::Base before_validation -> { self.arch_id = Arch.find_by(name: 'x86_64').id }, on: :create # field "not_delete" can be changed only if build has been completed before_validation -> { self.not_delete = false unless build_completed?; true } - validates :product_id, + validates :product, :status, - :project_id, + :project, :main_script, - :arch_id, presence: true + :arch, presence: true validates :status, inclusion: { in: STATUSES } validates :main_script, :params, length: { maximum: 255 } diff --git a/app/models/project.rb b/app/models/project.rb index d0ed6fa86..949245b8e 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -41,9 +41,9 @@ class Project < ActiveRecord::Base presence: true, format: { with: /\A#{NAME_REGEXP.source}\z/, message: I18n.t("activerecord.errors.project.uname") } - validates :maintainer_id, presence: true, unless: :new_record? + validates :maintainer, presence: true, unless: :new_record? validates :url, presence: true, format: { with: /\Ahttps?:\/\/[\S]+\z/ }, if: :mass_import - validates :add_to_repository_id, presence: true, if: :mass_import + validates :add_to_repository, presence: true, if: :mass_import validates :visibility, presence: true, inclusion: { in: VISIBILITIES } validate { errors.add(:base, :can_have_less_or_equal, count: MAX_OWN_PROJECTS) if owner.projects.size >= MAX_OWN_PROJECTS } validate :check_default_branch diff --git a/app/models/project_import.rb b/app/models/project_import.rb index c237b3c5f..5e9e886fa 100644 --- a/app/models/project_import.rb +++ b/app/models/project_import.rb @@ -3,7 +3,7 @@ class ProjectImport < ActiveRecord::Base belongs_to :platform validates :name, uniqueness: { scope: :platform_id, case_sensitive: false } - validates :name, :platform_id, :version, presence: true + validates :name, :platform, :version, presence: true scope :by_name, ->(name) { where("#{table_name}.name ILIKE ?", name) } diff --git a/app/models/project_statistic.rb b/app/models/project_statistic.rb index 5fe6716e6..11c971a83 100644 --- a/app/models/project_statistic.rb +++ b/app/models/project_statistic.rb @@ -3,7 +3,7 @@ class ProjectStatistic < ActiveRecord::Base belongs_to :arch belongs_to :project - validates :arch_id, :project_id, :average_build_time, :build_count, presence: true + validates :arch, :project, :average_build_time, :build_count, presence: true validates :project_id, uniqueness: { scope: :arch_id } attr_accessible :average_build_time, :build_count diff --git a/app/models/project_tag.rb b/app/models/project_tag.rb index 79ff43a7f..634fdffbb 100644 --- a/app/models/project_tag.rb +++ b/app/models/project_tag.rb @@ -8,7 +8,7 @@ class ProjectTag < ActiveRecord::Base belongs_to :project - validates :project_id, :commit_id, :sha1, :tag_name, :format_id, presence: true + validates :project, :commit, :sha1, :tag_name, :format_id, presence: true validates :project_id, uniqueness: { scope: [:tag_name, :format_id] } attr_accessible :project_id, :commit_id, :sha1, :tag_name, :format_id diff --git a/app/models/repository_status.rb b/app/models/repository_status.rb index bae86d1c3..f0d281d03 100644 --- a/app/models/repository_status.rb +++ b/app/models/repository_status.rb @@ -28,7 +28,7 @@ class RepositoryStatus < ActiveRecord::Base belongs_to :platform belongs_to :repository - validates :repository_id, :platform_id, presence: true + validates :repository, :platform, presence: true validates :repository_id, uniqueness: { scope: :platform_id } attr_accessible :platform_id, :repository_id diff --git a/app/models/settings_notifier.rb b/app/models/settings_notifier.rb index a531abe68..d54036853 100644 --- a/app/models/settings_notifier.rb +++ b/app/models/settings_notifier.rb @@ -1,7 +1,7 @@ class SettingsNotifier < ActiveRecord::Base belongs_to :user - validates :user_id, presence: true + validates :user, presence: true attr_accessible :can_notify, :update_code, @@ -9,7 +9,7 @@ class SettingsNotifier < ActiveRecord::Base :new_comment_commit_repo_owner, :new_comment_commit_commentor, :new_comment, - :new_comment_reply, + :new_comment_reply, :new_issue, :issue_assign, :new_build, From 212eb66a9eeaa7cfb0336f53d770a204da376e61 Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Tue, 25 Nov 2014 22:33:54 +0500 Subject: [PATCH 2/4] [#247] fix model specs --- app/models/build_list.rb | 10 +++++----- app/models/build_list/package.rb | 3 ++- app/models/build_script.rb | 3 ++- app/models/product_build_list.rb | 7 ++++--- spec/models/build_script_spec.rb | 2 +- spec/models/project_statistic_spec.rb | 4 ++-- 6 files changed, 16 insertions(+), 13 deletions(-) diff --git a/app/models/build_list.rb b/app/models/build_list.rb index fa7b6e35f..1e8609efd 100644 --- a/app/models/build_list.rb +++ b/app/models/build_list.rb @@ -38,13 +38,13 @@ class BuildList < ActiveRecord::Base AUTO_PUBLISH_STATUS_TESTING = 'testing' ] - validates :project, + validates :project, :project_id, :project_version, - :arch, + :arch, :arch_id, :include_repos, - :build_for_platform, - :save_to_platform, - :save_to_repository, + :build_for_platform, :build_for_platform_id, + :save_to_platform, :save_to_platform_id, + :save_to_repository, :save_to_repository_id, presence: true validates_numericality_of :priority, greater_than_or_equal_to: 0 validates :external_nodes, inclusion: { in: EXTERNAL_NODES }, allow_blank: true diff --git a/app/models/build_list/package.rb b/app/models/build_list/package.rb index 873c1227b..73120be63 100644 --- a/app/models/build_list/package.rb +++ b/app/models/build_list/package.rb @@ -9,7 +9,8 @@ class BuildList::Package < ActiveRecord::Base attr_accessible :fullname, :name, :release, :version, :sha1, :epoch, :dependent_packages - validates :build_list, :project, :platform, :fullname, + validates :build_list, :build_list_id, :project, :project_id, + :platform, :platform_id, :fullname, :package_type, :name, :release, :version, presence: true validates :package_type, inclusion: PACKAGE_TYPES diff --git a/app/models/build_script.rb b/app/models/build_script.rb index 30ebbb5b2..1c5a395d3 100644 --- a/app/models/build_script.rb +++ b/app/models/build_script.rb @@ -10,7 +10,8 @@ class BuildScript < ActiveRecord::Base belongs_to :project validates :treeish, presence: true - validates :project, presence: true, uniqueness: { scope: :treeish } + validates :project, presence: true + validates :project_id, uniqueness: { scope: :treeish } scope :by_active, -> { where(status: ACTIVE) } scope :by_treeish, -> treeish { where(treeish: treeish) } diff --git a/app/models/product_build_list.rb b/app/models/product_build_list.rb index 0c1d18a3f..f4a55a066 100644 --- a/app/models/product_build_list.rb +++ b/app/models/product_build_list.rb @@ -45,11 +45,12 @@ class ProductBuildList < ActiveRecord::Base before_validation -> { self.arch_id = Arch.find_by(name: 'x86_64').id }, on: :create # field "not_delete" can be changed only if build has been completed before_validation -> { self.not_delete = false unless build_completed?; true } - validates :product, + validates :product, :product_id, :status, - :project, + :project, :project_id, :main_script, - :arch, presence: true + :arch, :arch_id, + presence: true validates :status, inclusion: { in: STATUSES } validates :main_script, :params, length: { maximum: 255 } diff --git a/spec/models/build_script_spec.rb b/spec/models/build_script_spec.rb index c7fff54f5..30a8308db 100644 --- a/spec/models/build_script_spec.rb +++ b/spec/models/build_script_spec.rb @@ -12,7 +12,7 @@ describe BuildScript do context 'ensures that validations and associations exist' do it { should belong_to(:project) } - it { should validate_presence_of(:project_id) } + it { should validate_presence_of(:project) } it { should validate_presence_of(:treeish) } context 'uniqueness' do diff --git a/spec/models/project_statistic_spec.rb b/spec/models/project_statistic_spec.rb index e13a0ec20..fca55d974 100644 --- a/spec/models/project_statistic_spec.rb +++ b/spec/models/project_statistic_spec.rb @@ -6,8 +6,8 @@ describe ProjectStatistic do it { should belong_to(:project) } it { should belong_to(:arch) } - it { should validate_presence_of(:project_id) } - it { should validate_presence_of(:arch_id) } + it { should validate_presence_of(:project) } + it { should validate_presence_of(:arch) } it { should validate_presence_of(:average_build_time) } it { should validate_presence_of(:build_count) } From 697433eef50fc124def338c2921a0c9fabc67e39 Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Wed, 26 Nov 2014 14:59:21 +0500 Subject: [PATCH 3/4] [#247] add some api specs --- .../api/v1/build_lists_controller_spec.rb | 12 ++++++++++++ spec/controllers/api/v1/projects_controller_spec.rb | 6 +++++- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/spec/controllers/api/v1/build_lists_controller_spec.rb b/spec/controllers/api/v1/build_lists_controller_spec.rb index df5c5d01c..53ddd5fef 100644 --- a/spec/controllers/api/v1/build_lists_controller_spec.rb +++ b/spec/controllers/api/v1/build_lists_controller_spec.rb @@ -51,6 +51,18 @@ shared_examples_for 'create build list via api' do it 'should not create without existing commit hash in project' do lambda{ post :create, @create_params.deep_merge(build_list: {commit_hash: 'wrong'})}.should change{@project.build_lists.count}.by(0) end + + it 'should not create without existing arch' do + lambda{ post :create, @create_params.deep_merge(build_list: {arch_id: -1})}.should change{@project.build_lists.count}.by(0) + end + + it 'should not create without existing save_to_platform' do + lambda{ post :create, @create_params.deep_merge(build_list: {save_to_platform_id: -1})}.should change{@project.build_lists.count}.by(0) + end + + it 'should not create without existing save_to_repository' do + lambda{ post :create, @create_params.deep_merge(build_list: {save_to_repository_id: -1})}.should change{@project.build_lists.count}.by(0) + end end shared_examples_for 'not create build list via api' do diff --git a/spec/controllers/api/v1/projects_controller_spec.rb b/spec/controllers/api/v1/projects_controller_spec.rb index 06dcaa8ec..1fe487702 100644 --- a/spec/controllers/api/v1/projects_controller_spec.rb +++ b/spec/controllers/api/v1/projects_controller_spec.rb @@ -120,6 +120,10 @@ shared_examples_for 'api projects user with admin rights' do get :members, id: @project.id, format: :json response.should be_success end + it 'should not set a wrong maintainer_id' do + put :update, project: { maintainer_id: -1 }, id: @project.id, format: :json + response.should_not be_success + end context 'api project user with update rights' do before do @@ -129,7 +133,7 @@ shared_examples_for 'api projects user with admin rights' do it 'should be able to perform update action' do response.should be_success end - it 'ensures that group has been updated' do + it 'ensures that description has been updated' do @project.reload @project.description.should == 'new description' end From cd6e1c395621e8025d13214a41754e6e13c6934d Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Wed, 26 Nov 2014 17:22:55 +0500 Subject: [PATCH 4/4] [#247] fix build list spec --- spec/controllers/api/v1/build_lists_controller_spec.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/spec/controllers/api/v1/build_lists_controller_spec.rb b/spec/controllers/api/v1/build_lists_controller_spec.rb index 53ddd5fef..4b71c764e 100644 --- a/spec/controllers/api/v1/build_lists_controller_spec.rb +++ b/spec/controllers/api/v1/build_lists_controller_spec.rb @@ -57,7 +57,9 @@ shared_examples_for 'create build list via api' do end it 'should not create without existing save_to_platform' do - lambda{ post :create, @create_params.deep_merge(build_list: {save_to_platform_id: -1})}.should change{@project.build_lists.count}.by(0) + lambda{ + post :create, @create_params.deep_merge(build_list: {save_to_platform_id: -1, save_to_repository_id: -1}) + }.should change{@project.build_lists.count}.by(0) end it 'should not create without existing save_to_repository' do