Skip to content

Commit d099494

Browse files
koppenkoppen
committed
Clean up syntax
1 parent 995c32e commit d099494

File tree

3 files changed

+56
-19
lines changed

3 files changed

+56
-19
lines changed

‎app/services/github_hook/updater.rb‎

Lines changed: 20 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,8 @@ def exec(command, directory)
6464
logfile.unlink if logfile && logfile.respond_to?(:unlink)
6565
end
6666

67-
# Finds the Redmine project in the database based on the given project identifier
67+
# Finds the Redmine project in the database based on the given project
68+
# identifier
6869
def find_project
6970
identifier = get_identifier
7071
project = Project.find_by_identifier(identifier.downcase)
@@ -80,12 +81,15 @@ def find_repositories
8081
end
8182

8283
if repositories.nil? || repositories.length == 0
83-
fail TypeError, "Project '#{project}' ('#{project.identifier}') has no repository"
84+
fail(
85+
TypeError,
86+
"Project '#{project}' ('#{project.identifier}') has no repository"
87+
)
8488
end
8589

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

105-
# Gets the project identifier from the querystring parameters and if that's not supplied, assume
106-
# the Github repository name is the same as the project identifier.
109+
# Gets the project identifier from the querystring parameters and if that's
110+
# not supplied, assume the Github repository name is the same as the project
111+
# identifier.
107112
def get_identifier
108113
identifier = get_project_name
109114
fail ActiveRecord::RecordNotFound, "Project identifier not specified" if identifier.nil?
110115
identifier
111116
end
112117

113-
# Attempts to find the project name. It first looks in the params, then in the
114-
# payload if params[:project_id] isn't given.
118+
# Attempts to find the project name. It first looks in the params, then in
119+
# the payload if params[:project_id] isn't given.
115120
def get_project_name
116-
params[:project_id] || (payload["repository"] ? payload["repository"]["name"] : nil)
121+
project_id = params[:project_id]
122+
name_from_repository = payload.fetch("repository", {}).fetch("name", nil)
123+
project_id || name_from_repository
117124
end
118125

119126
def git_command(command)
@@ -136,7 +143,9 @@ def time_diff_milli(start, finish)
136143
def update_repository(repository)
137144
command = git_command("fetch origin")
138145
if exec(command, repository.url)
139-
command = git_command("fetch --prune origin \"+refs/heads/*:refs/heads/*\"")
146+
command = git_command(
147+
"fetch --prune origin \"+refs/heads/*:refs/heads/*\""
148+
)
140149
exec(command, repository.url)
141150
end
142151
end

‎test/functional/github_hook_controller_test.rb‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,10 @@ def test_should_render_response_from_github_hook_when_done
8484
end
8585

8686
def test_should_render_error_message
87-
GithubHook::Updater.any_instance.expects(:update_repository).raises(ActiveRecord::RecordNotFound.new("Repository not found"))
87+
GithubHook::Updater
88+
.any_instance
89+
.expects(:update_repository)
90+
.raises(ActiveRecord::RecordNotFound.new("Repository not found"))
8891
do_post
8992
assert_response :not_found
9093
assert_equal({

‎test/unit/github_hook/updater_test.rb‎

Lines changed: 32 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,21 @@ def repository
2020
@repository
2121
end
2222

23+
# rubocop:disable Metrics/LineLength
2324
def payload
2425
# Ruby hash with the parsed data from the JSON payload
25-
{"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"}
26-
end
26+
{
27+
"before" => "5aef35982fb2d34e9d9d4502f6ede1072793222d",
28+
"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"}},
29+
"commits" => [
30+
{"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"]},
31+
{"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"}
32+
],
33+
"after" => "de8251ff97ee194a289832576287d6f8ad74e3d0",
34+
"ref" => "refs/heads/master"
35+
}
36+
end
37+
# rubocop:enable Metrics/LineLength
2738

2839
def build_updater(payload, options = {})
2940
updater = GithubHook::Updater.new(payload, options)
@@ -59,14 +70,28 @@ def test_fetches_changes_from_origin
5970
end
6071

6172
def test_resets_repository_when_fetch_origin_succeeds
62-
updater.expects(:exec).with("git fetch origin", repository.url).returns(true)
63-
updater.expects(:exec).with("git fetch --prune origin \"+refs/heads/*:refs/heads/*\"", repository.url)
73+
updater
74+
.expects(:exec)
75+
.with("git fetch origin", repository.url)
76+
.returns(true)
77+
updater
78+
.expects(:exec)
79+
.with(
80+
"git fetch --prune origin \"+refs/heads/*:refs/heads/*\"",
81+
repository.url
82+
)
6483
updater.call
6584
end
6685

6786
def test_resets_repository_when_fetch_origin_fails
68-
updater.expects(:exec).with("git fetch origin", repository.url).returns(false)
69-
updater.expects(:exec).with("git reset --soft refs\/remotes\/origin\/master", repository.url).never
87+
updater
88+
.expects(:exec)
89+
.with("git fetch origin", repository.url)
90+
.returns(false)
91+
updater
92+
.expects(:exec)
93+
.with("git reset --soft refs\/remotes\/origin\/master", repository.url)
94+
.never
7095
updater.call
7196
end
7297

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

94-
updater = build_updater(payload, {:repository_id => "redmine"})
119+
updater = build_updater(payload, :repository_id => "redmine")
95120
updater.expects(:exec).with("git fetch origin", repository.url)
96121
updater.call
97122
end

0 commit comments

Comments
 (0)