From 1a09d0feade8d27c7b34cbec99008ced4c3372b1 Mon Sep 17 00:00:00 2001 From: Vokhmin Alexey V Date: Thu, 19 Feb 2015 02:34:07 +0300 Subject: [PATCH 1/2] #456: add default_branch to Group --- app/models/concerns/default_branchable.rb | 11 +++++++++++ app/models/group.rb | 1 + app/models/platform.rb | 7 +++---- app/models/project.rb | 3 ++- app/views/groups/profile/_form.html.slim | 1 + .../20150218231015_add_default_branch_to_groups.rb | 5 +++++ db/schema.rb | 3 ++- 7 files changed, 25 insertions(+), 6 deletions(-) create mode 100644 app/models/concerns/default_branchable.rb create mode 100644 db/migrate/20150218231015_add_default_branch_to_groups.rb diff --git a/app/models/concerns/default_branchable.rb b/app/models/concerns/default_branchable.rb new file mode 100644 index 000000000..ef66954a7 --- /dev/null +++ b/app/models/concerns/default_branchable.rb @@ -0,0 +1,11 @@ +module DefaultBranchable + extend ActiveSupport::Concern + + included do + validates :default_branch, + length: { maximum: 100 } + + attr_accessible :default_branch + end + +end diff --git a/app/models/group.rb b/app/models/group.rb index 0339c34c5..76a7ab2f2 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -1,6 +1,7 @@ class Group < Avatar include ActsLikeMember include PersonalRepository + include DefaultBranchable belongs_to :owner, class_name: 'User' diff --git a/app/models/platform.rb b/app/models/platform.rb index 8769cc82a..dcddaa29f 100644 --- a/app/models/platform.rb +++ b/app/models/platform.rb @@ -7,6 +7,7 @@ class Platform < ActiveRecord::Base include Owner include EventLoggable include EmptyMetadata + include DefaultBranchable self.per_page = 20 @@ -68,8 +69,7 @@ class Platform < ActiveRecord::Base length: { maximum: 100 } validates :default_branch, - presence: true, - length: { maximum: 100 } + presence: true validates :distrib_type, presence: true, @@ -118,8 +118,7 @@ class Platform < ActiveRecord::Base :platform_arch_settings_attributes, :automatic_metadata_regeneration, :admin_id, - :term, - :default_branch + :term attr_accessor :admin_id, :term diff --git a/app/models/project.rb b/app/models/project.rb index a2b41ffec..666937619 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -7,6 +7,7 @@ class Project < ActiveRecord::Base include Wiki include UrlHelper include EventLoggable + include DefaultBranchable VISIBILITIES = ['open', 'hidden'] MAX_OWN_PROJECTS = 32000 @@ -61,7 +62,7 @@ class Project < ActiveRecord::Base errors.delete :project_to_repositories end - attr_accessible :name, :description, :visibility, :srpm, :is_package, :default_branch, + attr_accessible :name, :description, :visibility, :srpm, :is_package, :has_issues, :has_wiki, :maintainer_id, :publish_i686_into_x86_64, :url, :srpms_list, :mass_import, :add_to_repository_id, :architecture_dependent, :autostart_status diff --git a/app/views/groups/profile/_form.html.slim b/app/views/groups/profile/_form.html.slim index e23371c1b..2429f27b9 100644 --- a/app/views/groups/profile/_form.html.slim +++ b/app/views/groups/profile/_form.html.slim @@ -2,5 +2,6 @@ - if [:new, :create].include? act = f.input :uname = f.input :description, as: :text += f.input :default_branch = render 'shared/avatar_form', subject: @group, f: f = submit_button_tag \ No newline at end of file diff --git a/db/migrate/20150218231015_add_default_branch_to_groups.rb b/db/migrate/20150218231015_add_default_branch_to_groups.rb new file mode 100644 index 000000000..ecb36ee7d --- /dev/null +++ b/db/migrate/20150218231015_add_default_branch_to_groups.rb @@ -0,0 +1,5 @@ +class AddDefaultBranchToGroups < ActiveRecord::Migration + def change + add_column :groups, :default_branch, :string + end +end diff --git a/db/schema.rb b/db/schema.rb index 586ebd187..51a4cfb3d 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20150217215529) do +ActiveRecord::Schema.define(version: 20150218231015) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -246,6 +246,7 @@ ActiveRecord::Schema.define(version: 20150217215529) do t.string "avatar_content_type" t.integer "avatar_file_size" t.datetime "avatar_updated_at" + t.string "default_branch" end create_table "hooks", force: true do |t| From 13cd2638556c10b84dabc27ecb347288ee4cf83b Mon Sep 17 00:00:00 2001 From: Vokhmin Alexey V Date: Fri, 20 Feb 2015 01:03:04 +0300 Subject: [PATCH 2/2] #456: add integration with Group#default_branch, specs --- .../projects/git/commits_controller.rb | 8 +- .../projects/git/trees_controller.rb | 2 +- .../projects/projects_controller.rb | 3 +- app/helpers/build_lists_helper.rb | 2 +- app/helpers/git_helper.rb | 4 +- app/models/concerns/project/default_branch.rb | 76 +++++++++++++++++++ app/models/project.rb | 35 +-------- config/locales/models/group.en.yml | 7 ++ config/locales/models/group.ru.yml | 8 ++ spec/models/project_spec.rb | 36 +++++++++ 10 files changed, 138 insertions(+), 43 deletions(-) create mode 100644 app/models/concerns/project/default_branch.rb diff --git a/app/controllers/projects/git/commits_controller.rb b/app/controllers/projects/git/commits_controller.rb index 80f1b7cb3..3dd5b5295 100644 --- a/app/controllers/projects/git/commits_controller.rb +++ b/app/controllers/projects/git/commits_controller.rb @@ -23,13 +23,13 @@ class Projects::Git::CommitsController < Projects::Git::BaseController res = params[:diff].split(/\A(.*)\.\.\.(.*)\z/).select {|e| e.present?} if res[1].present? params1 = res[0] - params2 = res[1] == 'HEAD' ? @project.default_branch : res.last + params2 = res[1] == 'HEAD' ? @project.resolve_default_branch : res.last else # get only one parameter - params1 = @project.default_branch + params1 = @project.resolve_default_branch params2 = res.first end - params1.sub! 'HEAD', @project.default_branch - params2.sub! 'HEAD', @project.default_branch + params1.sub! 'HEAD', @project.resolve_default_branch + params2.sub! 'HEAD', @project.resolve_default_branch ref1 = if @project.repo.branches_and_tags.include? params1 @project.repo.commits(params1).first diff --git a/app/controllers/projects/git/trees_controller.rb b/app/controllers/projects/git/trees_controller.rb index 3f7829b9f..4130a44da 100644 --- a/app/controllers/projects/git/trees_controller.rb +++ b/app/controllers/projects/git/trees_controller.rb @@ -1,6 +1,6 @@ class Projects::Git::TreesController < Projects::Git::BaseController - before_filter -> {redirect_to @project if params[:treeish] == @project.default_branch and params[:path].blank?}, only: :show + before_filter -> {redirect_to @project if params[:treeish] == @project.resolve_default_branch and params[:path].blank?}, only: :show skip_before_filter :set_branch_and_tree, :set_treeish_and_path, only: :archive before_filter -> { raise Grit::NoSuchPathError if params[:treeish] != @branch.try(:name) }, only: [:branch, :destroy] diff --git a/app/controllers/projects/projects_controller.rb b/app/controllers/projects/projects_controller.rb index a42e58577..7f220d307 100644 --- a/app/controllers/projects/projects_controller.rb +++ b/app/controllers/projects/projects_controller.rb @@ -173,7 +173,8 @@ class Projects::ProjectsController < Projects::BaseController def refs_list refs = @project.repo.branches_and_tags.map(&:name) - @selected = (refs.include? params[:selected]) ? params[:selected] : @project.default_branch + @selected = params[:selected] if refs.include?(params[:selected]) + @selected ||= @project.resolve_default_branch render layout: false end diff --git a/app/helpers/build_lists_helper.rb b/app/helpers/build_lists_helper.rb index 9641b06ae..30c370a66 100644 --- a/app/helpers/build_lists_helper.rb +++ b/app/helpers/build_lists_helper.rb @@ -257,7 +257,7 @@ module BuildListsHelper end def project_version(project, params) - @project_version ||= params[:build_list].try(:[], :project_version) || project.default_branch + @project_version ||= params[:build_list].try(:[], :project_version) || project.resolve_default_branch end def build_list_project_versions(project) diff --git a/app/helpers/git_helper.rb b/app/helpers/git_helper.rb index ebbb96b54..78e3a0f4d 100644 --- a/app/helpers/git_helper.rb +++ b/app/helpers/git_helper.rb @@ -23,7 +23,7 @@ module GitHelper def render_path # TODO: Looks ugly, rewrite with clear mind. if @path.present? - if @treeish == @project.default_branch + if @treeish == @project.resolve_default_branch res = "#{link_to @project.name, tree_path(@project)} / " else res = "#{link_to @project.name, tree_path(@project, @treeish)} / " @@ -70,7 +70,7 @@ module GitHelper def branch_selector_options(project) p, tag_enabled = params.dup, !(controller_name == 'trees' && action_name == 'branches') p.delete(:path) if p[:path].present? # to root path - p.merge!(project_id: project.id, treeish: project.default_branch).delete(:id) unless p[:treeish].present? + p.merge!(project_id: project.id, treeish: project.resolve_default_branch).delete(:id) unless p[:treeish].present? current = url_for(p).split('?', 2).first res = [] diff --git a/app/models/concerns/project/default_branch.rb b/app/models/concerns/project/default_branch.rb new file mode 100644 index 000000000..672c9d186 --- /dev/null +++ b/app/models/concerns/project/default_branch.rb @@ -0,0 +1,76 @@ +# Internal: various definitions and instance methods related to default_branch. +# +# This module gets mixed in into Project class. +module Project::DefaultBranch + extend ActiveSupport::Concern + + include DefaultBranchable + + included do + validate :check_default_branch + + after_update :set_new_git_head + end + + ###################################### + # Instance methods # + ###################################### + + # Public: Get default branch according to owner configs. + # + # Returns found String branch name. + def resolve_default_branch + return default_branch unless owner.is_a?(Group) && owner.default_branch.present? + return default_branch unless repo.branches.map(&:name).include?(owner.default_branch) + return owner.default_branch unless default_branch.present? + default_branch == 'master' ? owner.default_branch : default_branch + end + + # Public: Finds branch name for platforms. + # + # save_to_platform - The save Platform. + # build_for_platform - The build Platform. + # + # Returns found String branch name. + def project_version_for(save_to_platform, build_for_platform) + if repo.commits("#{save_to_platform.default_branch}").try(:first).try(:id) + save_to_platform.default_branch + elsif repo.commits("#{build_for_platform.default_branch}").try(:first).try(:id) + build_for_platform.default_branch + else + resolve_default_branch + end + end + + # Public: Finds default head. + # + # treeish - The String treeish. + # + # Returns found String head. + def default_head(treeish = nil) # maybe need change 'head'? + # Attention! + # repo.commit(nil) => + # repo.commit(nil.to_s) => nil + return treeish if treeish.present? && repo.commit(treeish).present? + if repo.branches_and_tags.map(&:name).include?(treeish || resolve_default_branch) + treeish || resolve_default_branch + else + repo.branches_and_tags[0].try(:name) || resolve_default_branch + end + end + + protected + + # Private: Set git head. + def set_new_git_head + `cd #{path} && git symbolic-ref HEAD refs/heads/#{self.default_branch}` if self.default_branch_changed? && self.repo.branches.map(&:name).include?(self.default_branch) + end + + # Private: Validation for checking that the default branch is exist. + def check_default_branch + if self.repo.branches.count > 0 && self.repo.branches.map(&:name).exclude?(self.default_branch) + errors.add :default_branch, I18n.t('activerecord.errors.project.default_branch') + end + end + +end diff --git a/app/models/project.rb b/app/models/project.rb index 666937619..ccd5fbed3 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -7,7 +7,7 @@ class Project < ActiveRecord::Base include Wiki include UrlHelper include EventLoggable - include DefaultBranchable + include Project::DefaultBranch VISIBILITIES = ['open', 'hidden'] MAX_OWN_PROJECTS = 32000 @@ -52,7 +52,6 @@ class Project < ActiveRecord::Base 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 # throws validation error message from ProjectToRepository model into Project model validate do |project| project.project_to_repositories.each do |p_to_r| @@ -98,7 +97,6 @@ class Project < ActiveRecord::Base before_save -> { self.owner_uname = owner.uname if owner_uname.blank? || owner_id_changed? || owner_type_changed? } before_create :set_maintainer after_save :attach_to_personal_repository - after_update :set_new_git_head after_update -> { update_path_to_project(name_was) }, if: :name_changed? attr_accessor :url, :srpms_list, :mass_import, :add_to_repository_id @@ -210,16 +208,6 @@ class Project < ActiveRecord::Base build_list.save end - def project_version_for(save_to_platform, build_for_platform) - if repo.commits("#{save_to_platform.default_branch}").try(:first).try(:id) - save_to_platform.default_branch - elsif repo.commits("#{build_for_platform.default_branch}").try(:first).try(:id) - build_for_platform.default_branch - else - default_branch - end - end - def fork(new_owner, new_name: nil, is_alias: false) new_name = new_name.presence || name dup.tap do |c| @@ -234,18 +222,6 @@ class Project < ActiveRecord::Base end end - def default_head treeish = nil # maybe need change 'head'? - # Attention! - # repo.commit(nil) => - # repo.commit(nil.to_s) => nil - return treeish if treeish.present? && repo.commit(treeish).present? - if repo.branches_and_tags.map(&:name).include?(treeish || default_branch) - treeish || default_branch - else - repo.branches_and_tags[0].try(:name) || default_branch - end - end - def get_project_tag_sha1(tag, format) format_id = ProjectTag::FORMATS["#{tag_file_format(format)}"] project_tag = project_tags.where(tag_name: tag.name, format_id: format_id).first @@ -395,10 +371,6 @@ class Project < ActiveRecord::Base end end - def set_new_git_head - `cd #{path} && git symbolic-ref HEAD refs/heads/#{self.default_branch}` if self.default_branch_changed? && self.repo.branches.map(&:name).include?(self.default_branch) - end - def update_path_to_project(old_name) new_name, new_path = name, path self.name = old_name @@ -420,9 +392,4 @@ class Project < ActiveRecord::Base end later :update_path_to_project, queue: :middle - def check_default_branch - if self.repo.branches.count > 0 && self.repo.branches.map(&:name).exclude?(self.default_branch) - errors.add :default_branch, I18n.t('activerecord.errors.project.default_branch') - end - end end diff --git a/config/locales/models/group.en.yml b/config/locales/models/group.en.yml index 25ada7e9c..0db83786d 100644 --- a/config/locales/models/group.en.yml +++ b/config/locales/models/group.en.yml @@ -54,7 +54,14 @@ en: description: Description avatar: Avatar delete_avatar: Delete avatar + default_branch: Default branch + hints: + group: + default_branch: > + This is the name of branch that will be used by default for all projects. + Ignored, if project already has a default branch different from master. placeholders: group: uname: Enter the name here. description: Enter the description here. + default_branch: Enter the name of branch here. diff --git a/config/locales/models/group.ru.yml b/config/locales/models/group.ru.yml index d2193e401..c2160682e 100644 --- a/config/locales/models/group.ru.yml +++ b/config/locales/models/group.ru.yml @@ -54,7 +54,15 @@ ru: avatar: Аватар delete_avatar: Удалить аватар description: Описание + default_branch: Ветка по умолчанию + hints: + group: + default_branch: > + Это имя ветки, которая будет использоваться по умолчанию для всех проектов. + Не учитывается, если в проекте уже установлена ветка по умолчанию, + отличная от master. placeholders: group: uname: Введите название здесь. description: Введите описание здесь. + default_branch: Введите имя ветки здесь. diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index aa8d05835..51a734d23 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -337,4 +337,40 @@ describe Project do it_should_behave_like 'autostart build_lists', 0, 0, 6 end end + + context '#resolve_default_branch' do + let(:project) { FactoryGirl.build(:project) } + let(:group_project) { FactoryGirl.build(:group_project) } + + it 'returns project default branch if owner is User' do + expect(project.resolve_default_branch).to eq 'master' + end + + it 'returns project default branch if Group has no default branch' do + expect(group_project.resolve_default_branch).to eq 'master' + end + + it 'returns project default branch if Group default branch does not exist in project' do + group_project.owner.default_branch = 'rosa' + expect(group_project.repo).to receive(:branches).and_return([]) + expect(group_project.resolve_default_branch).to eq 'master' + end + + context 'default branch of Group exists in project' do + before do + group_project.owner.default_branch = 'rosa' + expect(group_project.repo).to receive(:branches).and_return([double(:branch, name: 'rosa')]) + end + + it 'returns Group default branch' do + expect(group_project.resolve_default_branch).to eq 'rosa' + end + + it 'returns project default branch if it not equal to master' do + group_project.default_branch = 'cooker' + expect(group_project.resolve_default_branch).to eq 'cooker' + end + end + + end end