Commit 541d89941014137762dff696c83b3357eba8efeb
1 parent
49b024f5
Exists in
master
and in
4 other branches
Project.repository should never be nil so you can call repository.exists? or repository.empty?
Also specify separate project factory for project with filled repo
Showing
13 changed files
with
51 additions
and
54 deletions
Show diff stats
app/helpers/application_helper.rb
... | ... | @@ -96,7 +96,7 @@ module ApplicationHelper |
96 | 96 | ] |
97 | 97 | |
98 | 98 | project_nav = [] |
99 | - if @project && @project.repository && @project.repository.root_ref | |
99 | + if @project && @project.repository.exists? && @project.repository.root_ref | |
100 | 100 | project_nav = [ |
101 | 101 | { label: "#{simple_sanitize(@project.name_with_namespace)} - Issues", url: project_issues_path(@project) }, |
102 | 102 | { label: "#{simple_sanitize(@project.name_with_namespace)} - Commits", url: project_commits_path(@project, @ref || @project.repository.root_ref) }, | ... | ... |
app/models/project.rb
... | ... | @@ -141,13 +141,7 @@ class Project < ActiveRecord::Base |
141 | 141 | end |
142 | 142 | |
143 | 143 | def repository |
144 | - if path | |
145 | - @repository ||= Repository.new(path_with_namespace, default_branch) | |
146 | - else | |
147 | - nil | |
148 | - end | |
149 | - rescue Gitlab::Git::NoRepository | |
150 | - nil | |
144 | + @repository ||= Repository.new(path_with_namespace, default_branch) | |
151 | 145 | end |
152 | 146 | |
153 | 147 | def saved? |
... | ... | @@ -332,14 +326,14 @@ class Project < ActiveRecord::Base |
332 | 326 | end |
333 | 327 | |
334 | 328 | def valid_repo? |
335 | - repo | |
329 | + repository.exists? | |
336 | 330 | rescue |
337 | 331 | errors.add(:path, "Invalid repository path") |
338 | 332 | false |
339 | 333 | end |
340 | 334 | |
341 | 335 | def empty_repo? |
342 | - !repository || repository.empty? | |
336 | + !repository.exists? || repository.empty? | |
343 | 337 | end |
344 | 338 | |
345 | 339 | def ensure_satellite_exists |
... | ... | @@ -363,7 +357,7 @@ class Project < ActiveRecord::Base |
363 | 357 | end |
364 | 358 | |
365 | 359 | def repo_exists? |
366 | - @repo_exists ||= (repository && repository.branches.present?) | |
360 | + @repo_exists ||= repository.exists? | |
367 | 361 | rescue |
368 | 362 | @repo_exists = false |
369 | 363 | end | ... | ... |
app/models/repository.rb
... | ... | @@ -3,6 +3,16 @@ class Repository |
3 | 3 | |
4 | 4 | def initialize(path_with_namespace, default_branch) |
5 | 5 | @raw_repository = Gitlab::Git::Repository.new(path_with_namespace, default_branch) |
6 | + rescue Gitlab::Git::Repository::NoRepository | |
7 | + nil | |
8 | + end | |
9 | + | |
10 | + def exists? | |
11 | + raw_repository | |
12 | + end | |
13 | + | |
14 | + def empty? | |
15 | + raw_repository.empty? | |
6 | 16 | end |
7 | 17 | |
8 | 18 | def commit(id = nil) | ... | ... |
app/views/projects/_clone_panel.html.haml
... | ... | @@ -4,7 +4,7 @@ |
4 | 4 | .form-horizontal= render "shared/clone_panel" |
5 | 5 | .span4.pull-right |
6 | 6 | .pull-right |
7 | - - unless @project.empty_repo? | |
7 | + - if @project.empty_repo? | |
8 | 8 | - if can? current_user, :download_code, @project |
9 | 9 | = link_to archive_project_repository_path(@project), class: "btn-small btn grouped" do |
10 | 10 | %i.icon-download-alt | ... | ... |
features/steps/shared/project.rb
... | ... | @@ -3,14 +3,14 @@ module SharedProject |
3 | 3 | |
4 | 4 | # Create a project without caring about what it's called |
5 | 5 | And "I own a project" do |
6 | - @project = create(:project) | |
6 | + @project = create(:project_with_code) | |
7 | 7 | @project.team << [@user, :master] |
8 | 8 | end |
9 | 9 | |
10 | 10 | # Create a specific project called "Shop" |
11 | 11 | And 'I own project "Shop"' do |
12 | 12 | @project = Project.find_by_name "Shop" |
13 | - @project ||= create(:project, name: "Shop") | |
13 | + @project ||= create(:project_with_code, name: "Shop") | |
14 | 14 | @project.team << [@user, :master] |
15 | 15 | end |
16 | 16 | ... | ... |
lib/api/projects.rb
... | ... | @@ -372,7 +372,7 @@ module Gitlab |
372 | 372 | ref = params[:ref_name] || user_project.try(:default_branch) || 'master' |
373 | 373 | |
374 | 374 | commits = user_project.repository.commits(ref, nil, per_page, page * per_page) |
375 | - present CommitDecorator.decorate(commits), with: Entities::RepoCommit | |
375 | + present commits, with: Entities::RepoCommit | |
376 | 376 | end |
377 | 377 | |
378 | 378 | # Get a project snippets | ... | ... |
lib/extracts_path.rb
... | ... | @@ -85,8 +85,8 @@ module ExtractsPath |
85 | 85 | # - @id - A string representing the joined ref and path |
86 | 86 | # - @ref - A string representing the ref (e.g., the branch, tag, or commit SHA) |
87 | 87 | # - @path - A string representing the filesystem path |
88 | - # - @commit - A CommitDecorator representing the commit from the given ref | |
89 | - # - @tree - A TreeDecorator representing the tree at the given ref/path | |
88 | + # - @commit - A Commit representing the commit from the given ref | |
89 | + # - @tree - A Tree representing the tree at the given ref/path | |
90 | 90 | # |
91 | 91 | # If the :id parameter appears to be requesting a specific response format, |
92 | 92 | # that will be handled as well. | ... | ... |
lib/gitlab/markdown.rb
... | ... | @@ -187,7 +187,7 @@ module Gitlab |
187 | 187 | |
188 | 188 | def reference_commit(identifier) |
189 | 189 | if @project.valid_repo? && commit = @project.repository.commit(identifier) |
190 | - link_to(identifier, project_commit_url(@project, commit), html_options.merge(title: CommitDecorator.new(commit).link_title, class: "gfm gfm-commit #{html_options[:class]}")) | |
190 | + link_to(identifier, project_commit_url(@project, commit), html_options.merge(title: commit.link_title, class: "gfm gfm-commit #{html_options[:class]}")) | |
191 | 191 | end |
192 | 192 | end |
193 | 193 | end | ... | ... |
spec/factories.rb
... | ... | @@ -25,7 +25,7 @@ FactoryGirl.define do |
25 | 25 | |
26 | 26 | factory :project do |
27 | 27 | sequence(:name) { |n| "project#{n}" } |
28 | - path { 'gitlabhq' } | |
28 | + path { name.downcase.gsub(/\s/, '_') } | |
29 | 29 | creator |
30 | 30 | end |
31 | 31 | |
... | ... | @@ -34,6 +34,10 @@ FactoryGirl.define do |
34 | 34 | issues_tracker_id { "project_name_in_redmine" } |
35 | 35 | end |
36 | 36 | |
37 | + factory :project_with_code, parent: :project do | |
38 | + path { 'gitlabhq' } | |
39 | + end | |
40 | + | |
37 | 41 | factory :group do |
38 | 42 | sequence(:name) { |n| "group#{n}" } |
39 | 43 | path { name.downcase.gsub(/\s/, '_') } | ... | ... |
spec/helpers/gitlab_markdown_helper_spec.rb
... | ... | @@ -7,7 +7,7 @@ describe GitlabMarkdownHelper do |
7 | 7 | let!(:project) { create(:project) } |
8 | 8 | |
9 | 9 | let(:user) { create(:user, username: 'gfm') } |
10 | - let(:commit) { CommitDecorator.decorate(Commit.new(project.repository.commit)) } | |
10 | + let(:commit) { project.repository.commit) } | |
11 | 11 | let(:issue) { create(:issue, project: project) } |
12 | 12 | let(:merge_request) { create(:merge_request, project: project) } |
13 | 13 | let(:snippet) { create(:snippet, project: project) } | ... | ... |
spec/models/commit_spec.rb
... | ... | @@ -3,35 +3,32 @@ require 'spec_helper' |
3 | 3 | describe Commit do |
4 | 4 | let(:commit) { create(:project).repository.commit } |
5 | 5 | |
6 | - describe CommitDecorator do | |
7 | - let(:decorator) { CommitDecorator.new(commit) } | |
8 | 6 | |
9 | - describe '#title' do | |
10 | - it "returns no_commit_message when safe_message is blank" do | |
11 | - decorator.stub(:safe_message).and_return('') | |
12 | - decorator.title.should == "--no commit message" | |
13 | - end | |
7 | + describe '#title' do | |
8 | + it "returns no_commit_message when safe_message is blank" do | |
9 | + commit.stub(:safe_message).and_return('') | |
10 | + commit.title.should == "--no commit message" | |
11 | + end | |
14 | 12 | |
15 | - it "truncates a message without a newline at 70 characters" do | |
16 | - message = commit.safe_message * 10 | |
13 | + it "truncates a message without a newline at 70 characters" do | |
14 | + message = commit.safe_message * 10 | |
17 | 15 | |
18 | - decorator.stub(:safe_message).and_return(message) | |
19 | - decorator.title.should == "#{message[0..69]}…" | |
20 | - end | |
16 | + commit.stub(:safe_message).and_return(message) | |
17 | + commit.title.should == "#{message[0..69]}…" | |
18 | + end | |
21 | 19 | |
22 | - it "truncates a message with a newline before 80 characters at the newline" do | |
23 | - message = commit.safe_message.split(" ").first | |
20 | + it "truncates a message with a newline before 80 characters at the newline" do | |
21 | + message = commit.safe_message.split(" ").first | |
24 | 22 | |
25 | - decorator.stub(:safe_message).and_return(message + "\n" + message) | |
26 | - decorator.title.should == message | |
27 | - end | |
23 | + commit.stub(:safe_message).and_return(message + "\n" + message) | |
24 | + commit.title.should == message | |
25 | + end | |
28 | 26 | |
29 | - it "truncates a message with a newline after 80 characters at 70 characters" do | |
30 | - message = (commit.safe_message * 10) + "\n" | |
27 | + it "truncates a message with a newline after 80 characters at 70 characters" do | |
28 | + message = (commit.safe_message * 10) + "\n" | |
31 | 29 | |
32 | - decorator.stub(:safe_message).and_return(message) | |
33 | - decorator.title.should == "#{message[0..69]}…" | |
34 | - end | |
30 | + commit.stub(:safe_message).and_return(message) | |
31 | + commit.title.should == "#{message[0..69]}…" | |
35 | 32 | end |
36 | 33 | end |
37 | 34 | ... | ... |
spec/models/project_spec.rb
... | ... | @@ -119,7 +119,7 @@ describe Project do |
119 | 119 | end |
120 | 120 | |
121 | 121 | describe :update_merge_requests do |
122 | - let(:project) { create(:project) } | |
122 | + let(:project) { create(:project_with_code) } | |
123 | 123 | |
124 | 124 | before do |
125 | 125 | @merge_request = create(:merge_request, project: project) |
... | ... | @@ -187,11 +187,7 @@ describe Project do |
187 | 187 | let(:project) { create(:project) } |
188 | 188 | |
189 | 189 | it "should return valid repo" do |
190 | - project.repository.should be_kind_of(Gitlab::Git::Repository) | |
191 | - end | |
192 | - | |
193 | - it "should return nil" do | |
194 | - Project.new(path: "empty").repository.should be_nil | |
190 | + project.repository.should be_kind_of(Repository) | |
195 | 191 | end |
196 | 192 | end |
197 | 193 | |
... | ... | @@ -249,7 +245,7 @@ describe Project do |
249 | 245 | end |
250 | 246 | |
251 | 247 | describe :open_branches do |
252 | - let(:project) { create(:project) } | |
248 | + let(:project) { create(:project_with_code) } | |
253 | 249 | |
254 | 250 | before do |
255 | 251 | project.protected_branches.create(name: 'master') | ... | ... |
spec/spec_helper.rb
... | ... | @@ -47,11 +47,7 @@ Spork.prefork do |
47 | 47 | config.use_transactional_fixtures = false |
48 | 48 | |
49 | 49 | config.before do |
50 | - # Use tmp dir for FS manipulations | |
51 | - temp_repos_path = Rails.root.join('tmp', 'test-git-base-path') | |
52 | - Gitlab.config.gitlab_shell.stub(repos_path: temp_repos_path) | |
53 | - FileUtils.rm_rf temp_repos_path | |
54 | - FileUtils.mkdir_p temp_repos_path | |
50 | + TestEnv.init | |
55 | 51 | end |
56 | 52 | end |
57 | 53 | end | ... | ... |