From 13cd2638556c10b84dabc27ecb347288ee4cf83b Mon Sep 17 00:00:00 2001 From: Vokhmin Alexey V Date: Fri, 20 Feb 2015 01:03:04 +0300 Subject: [PATCH] #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