Skip to content

Commit

Permalink
Clean up syntax
Browse files Browse the repository at this point in the history
  • Loading branch information
@koppen
koppen committed Oct 18, 2015
1 parent 995c32e commit d099494
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 19 deletions.
31 changes: 20 additions & 11 deletions app/services/github_hook/updater.rb
View file
Expand Up @@ -64,7 +64,8 @@ def exec(command, directory)
logfile.unlink if logfile && logfile.respond_to?(:unlink)
end

# Finds the Redmine project in the database based on the given project identifier
# Finds the Redmine project in the database based on the given project
# identifier
def find_project
identifier = get_identifier
project = Project.find_by_identifier(identifier.downcase)
Expand All @@ -80,12 +81,15 @@ def find_repositories
end

if repositories.nil? || repositories.length == 0
fail TypeError, "Project '#{project}' ('#{project.identifier}') has no repository"
fail(
TypeError,
"Project '#{project}' ('#{project.identifier}') has no repository"
)
end

# if a specific repository id is passed in url parameter "repository_id", then try to find it in
# the list of current project repositories and use only this and not all to pull changes from
# (issue #54)
# if a specific repository id is passed in url parameter "repository_id",
# then try to find it in the list of current project repositories and use
# only this and not all to pull changes from (issue #54)
if params.key?(:repository_id)
param_repo = repositories.select do |repo|
repo.identifier == params[:repository_id]
Expand All @@ -102,18 +106,21 @@ def find_repositories
repositories
end

# Gets the project identifier from the querystring parameters and if that's not supplied, assume
# the Github repository name is the same as the project identifier.
# Gets the project identifier from the querystring parameters and if that's
# not supplied, assume the Github repository name is the same as the project
# identifier.
def get_identifier
identifier = get_project_name
fail ActiveRecord::RecordNotFound, "Project identifier not specified" if identifier.nil?
identifier
end

# Attempts to find the project name. It first looks in the params, then in the
# payload if params[:project_id] isn't given.
# Attempts to find the project name. It first looks in the params, then in
# the payload if params[:project_id] isn't given.
def get_project_name
params[:project_id] || (payload["repository"] ? payload["repository"]["name"] : nil)
project_id = params[:project_id]
name_from_repository = payload.fetch("repository", {}).fetch("name", nil)
project_id || name_from_repository
end

def git_command(command)
Expand All @@ -136,7 +143,9 @@ def time_diff_milli(start, finish)
def update_repository(repository)
command = git_command("fetch origin")
if exec(command, repository.url)
command = git_command("fetch --prune origin \"+refs/heads/*:refs/heads/*\"")
command = git_command(
"fetch --prune origin \"+refs/heads/*:refs/heads/*\""
)
exec(command, repository.url)
end
end
Expand Down
5 changes: 4 additions & 1 deletion test/functional/github_hook_controller_test.rb
View file
Expand Up @@ -84,7 +84,10 @@ def test_should_render_response_from_github_hook_when_done
end

def test_should_render_error_message
GithubHook::Updater.any_instance.expects(:update_repository).raises(ActiveRecord::RecordNotFound.new("Repository not found"))
GithubHook::Updater
.any_instance
.expects(:update_repository)
.raises(ActiveRecord::RecordNotFound.new("Repository not found"))
do_post
assert_response :not_found
assert_equal({
Expand Down
39 changes: 32 additions & 7 deletions test/unit/github_hook/updater_test.rb
View file
Expand Up @@ -20,10 +20,21 @@ def repository
@repository
end

# rubocop:disable Metrics/LineLength
def payload
# Ruby hash with the parsed data from the JSON payload
{"before" => "5aef35982fb2d34e9d9d4502f6ede1072793222d", "repository" => {"url" => "http://github.com/defunkt/github", "name" => "github", "description" => "You're lookin' at it.", "watchers" => 5, "forks" => 2, "private" => 1, "owner" => {"email" => "chris@ozmm.org", "name" => "defunkt"}}, "commits" => [{"id" => "41a212ee83ca127e3c8cf465891ab7216a705f59", "url" => "http://github.com/defunkt/github/commit/41a212ee83ca127e3c8cf465891ab7216a705f59", "author" => {"email" => "chris@ozmm.org", "name" => "Chris Wanstrath"}, "message" => "okay i give in", "timestamp" => "2008-02-15T14:57:17-08:00", "added" => ["filepath.rb"]}, {"id" => "de8251ff97ee194a289832576287d6f8ad74e3d0", "url" => "http://github.com/defunkt/github/commit/de8251ff97ee194a289832576287d6f8ad74e3d0", "author" => {"email" => "chris@ozmm.org", "name" => "Chris Wanstrath"}, "message" => "update pricing a tad", "timestamp" => "2008-02-15T14:36:34-08:00"}], "after" => "de8251ff97ee194a289832576287d6f8ad74e3d0", "ref" => "refs/heads/master"}
end
{
"before" => "5aef35982fb2d34e9d9d4502f6ede1072793222d",
"repository" => {"url" => "http://github.com/defunkt/github", "name" => "github", "description" => "You're lookin' at it.", "watchers" => 5, "forks" => 2, "private" => 1, "owner" => {"email" => "chris@ozmm.org", "name" => "defunkt"}},
"commits" => [
{"id" => "41a212ee83ca127e3c8cf465891ab7216a705f59", "url" => "http://github.com/defunkt/github/commit/41a212ee83ca127e3c8cf465891ab7216a705f59", "author" => {"email" => "chris@ozmm.org", "name" => "Chris Wanstrath"}, "message" => "okay i give in", "timestamp" => "2008-02-15T14:57:17-08:00", "added" => ["filepath.rb"]},
{"id" => "de8251ff97ee194a289832576287d6f8ad74e3d0", "url" => "http://github.com/defunkt/github/commit/de8251ff97ee194a289832576287d6f8ad74e3d0", "author" => {"email" => "chris@ozmm.org", "name" => "Chris Wanstrath"}, "message" => "update pricing a tad", "timestamp" => "2008-02-15T14:36:34-08:00"}
],
"after" => "de8251ff97ee194a289832576287d6f8ad74e3d0",
"ref" => "refs/heads/master"
}
end
# rubocop:enable Metrics/LineLength

def build_updater(payload, options = {})
updater = GithubHook::Updater.new(payload, options)
Expand Down Expand Up @@ -59,14 +70,28 @@ def test_fetches_changes_from_origin
end

def test_resets_repository_when_fetch_origin_succeeds
updater.expects(:exec).with("git fetch origin", repository.url).returns(true)
updater.expects(:exec).with("git fetch --prune origin \"+refs/heads/*:refs/heads/*\"", repository.url)
updater
.expects(:exec)
.with("git fetch origin", repository.url)
.returns(true)
updater
.expects(:exec)
.with(
"git fetch --prune origin \"+refs/heads/*:refs/heads/*\"",
repository.url
)
updater.call
end

def test_resets_repository_when_fetch_origin_fails
updater.expects(:exec).with("git fetch origin", repository.url).returns(false)
updater.expects(:exec).with("git reset --soft refs\/remotes\/origin\/master", repository.url).never
updater
.expects(:exec)
.with("git fetch origin", repository.url)
.returns(false)
updater
.expects(:exec)
.with("git reset --soft refs\/remotes\/origin\/master", repository.url)
.never
updater.call
end

Expand All @@ -91,7 +116,7 @@ def test_updates_only_the_specified_repository
another_repository.expects(:fetch_changesets).never
project.repositories << another_repository

updater = build_updater(payload, {:repository_id => "redmine"})
updater = build_updater(payload, :repository_id => "redmine")
updater.expects(:exec).with("git fetch origin", repository.url)
updater.call
end
Expand Down

0 comments on commit d099494

Please sign in to comment.