From ed26ecae0c3303b5554b033abd6f0a078b7573c0 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Sun, 16 Sep 2012 16:21:46 -0400 Subject: [PATCH 01/65] Add branches method to Repository role --- app/roles/repository.rb | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/app/roles/repository.rb b/app/roles/repository.rb index 01156ac1..7118f156 100644 --- a/app/roles/repository.rb +++ b/app/roles/repository.rb @@ -45,8 +45,16 @@ module Repository File.exists?(hook_file) end + def branches + repo.branches.collect(&:name).sort + end + def tags - repo.tags.map(&:name).sort.reverse + repo.tags.collect(&:name).sort.reverse + end + + def ref_names + [branches + tags].flatten end def repo @@ -79,14 +87,6 @@ module Repository @heads ||= repo.heads end - def branches_names - heads.map(&:name) - end - - def ref_names - [branches_names + tags].flatten - end - def tree(fcommit, path = nil) fcommit = commit if fcommit == :head tree = fcommit.tree @@ -109,8 +109,6 @@ module Repository # - If two or more branches are present, returns the one that has a name # matching root_ref (default_branch or 'master' if default_branch is nil) def discover_default_branch - branches = heads.collect(&:name) - if branches.length == 0 nil elsif branches.length == 1 From 5e1ef575df927a1132e8991f7d5dcc2f43217456 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Mon, 17 Sep 2012 10:08:02 -0400 Subject: [PATCH 02/65] Add CommitController --- app/controllers/commit_controller.rb | 33 ++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100644 app/controllers/commit_controller.rb diff --git a/app/controllers/commit_controller.rb b/app/controllers/commit_controller.rb new file mode 100644 index 00000000..73a55614 --- /dev/null +++ b/app/controllers/commit_controller.rb @@ -0,0 +1,33 @@ +# Controller for a specific Commit +# +# Not to be confused with CommitsController, plural. +class CommitController < ApplicationController + before_filter :project + layout "project" + + # Authorize + before_filter :add_project_abilities + before_filter :authorize_read_project! + before_filter :authorize_code_access! + before_filter :require_non_empty_project + + def show + result = CommitLoad.new(project, current_user, params).execute + + @commit = result[:commit] + + if @commit + @suppress_diff = result[:suppress_diff] + @note = result[:note] + @line_notes = result[:line_notes] + @notes_count = result[:notes_count] + @comments_allowed = true + else + return git_not_found! + end + + if result[:status] == :huge_commit + render "huge_commit" and return + end + end +end From bde5088525512643434ab88b491c0ba9ad6e248b Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Sun, 16 Sep 2012 21:14:21 -0400 Subject: [PATCH 03/65] Add routing specs for new routes --- spec/routing/project_routing_spec.rb | 59 ++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/spec/routing/project_routing_spec.rb b/spec/routing/project_routing_spec.rb index b3f9db01..d4ee5c33 100644 --- a/spec/routing/project_routing_spec.rb +++ b/spec/routing/project_routing_spec.rb @@ -396,3 +396,62 @@ describe NotesController, "routing" do let(:controller) { 'notes' } end end + +# TODO: Pending +# +# /:project_id/blame/*path +# /gitlabhq/blame/master/app/contexts/base_context.rb +# /gitlabhq/blame/test/branch/name/app/contexts/base_context.rb +# +# /:project_id/blob/*path +# /gitlabhq/blob/master/app/contexts/base_context.rb +# /gitlabhq/blob/test/branch/name/app/contexts/base_context.rb +# +# /:project_id/commit/:id +# /gitlabhq/commit/caef9ed1121a16ca0cc78715695daaa974271bfd +# +# /:project_id/commits +# +# /:project_id/commits/*path +# /gitlabhq/commits/master/app/contexts/base_context.rb +# /gitlabhq/commits/test/branch/name/app/contexts/base_context.rb +# +# /:project_id/raw/*path +# /gitlabhq/raw/master/app/contexts/base_context.rb +# /gitlabhq/raw/test/branch/name/app/contexts/base_context.rb +# +# /:project_id/tree/*path +# /gitlabhq/tree/master/app +# /gitlabhq/tree/test/branch/name/app +describe "pending routing" do + describe "/:project_id/blame/:id" do + it "routes to a ref with a path" do + get("/gitlabhq/blame/master/app/models/project.rb").should route_to('blame#show', project_id: 'gitlabhq', id: 'master/app/models/project.rb') + end + end + + describe "/:project_id/blob/:id" do + it "routes to a ref with a path" do + get("/gitlabhq/blob/master/app/models/project.rb").should route_to('blob#show', project_id: 'gitlabhq', id: 'master/app/models/project.rb') + end + end + + describe "/:project_id/commit/:id" do + it "routes to a specific commit" do + get("/gitlabhq/commit/f4b1449").should route_to('commit#show', project_id: 'gitlabhq', id: 'f4b1449') + get("/gitlabhq/commit/f4b14494ef6abf3d144c28e4af0c20143383e062").should route_to('commit#show', project_id: 'gitlabhq', id: 'f4b14494ef6abf3d144c28e4af0c20143383e062') + end + end + + describe "/:project_id/raw/:id" do + it "routes to a ref with a path" do + get("/gitlabhq/raw/master/app/models/project.rb").should route_to('raw#show', project_id: 'gitlabhq', id: 'master/app/models/project.rb') + end + end + + describe "/:project_id/tree/:id" do + it "routes to a ref with a path" do + get("/gitlabhq/tree/master/app/models/project.rb").should route_to('tree#show', project_id: 'gitlabhq', id: 'master/app/models/project.rb') + end + end +end From a21abce94f25d24b48c038e4a36974735a5b7149 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Mon, 17 Sep 2012 10:06:03 -0400 Subject: [PATCH 04/65] Add tree-ish route placeholders, modify commit(s) routes --- config/routes.rb | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/config/routes.rb b/config/routes.rb index cfb9bdb9..4f55d7e5 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -182,7 +182,10 @@ Gitlab::Application.routes.draw do get :test end end - resources :commits do + + resources :commit, only: [:show], constraints: {id: /[[:alnum:]]{6,40}/} + + resources :commits, only: [:index, :show] do collection do get :compare end @@ -191,6 +194,7 @@ Gitlab::Application.routes.draw do get :patch end end + resources :team, controller: 'team_members', only: [:index] resources :team_members resources :milestones @@ -208,6 +212,12 @@ Gitlab::Application.routes.draw do post :preview end end + + # XXX: WIP + # resources :blame, only: [:show], constraints: {id: /.+/} + # resources :blob, only: [:show], constraints: {id: /.+/} + # resources :raw, only: [:show], constraints: {id: /.+/} + # resources :tree, only: [:show], constraints: {id: /.+/} end root to: "dashboard#index" From b389247c029b21f5e85abb5896d2cf22230c9cb1 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Mon, 17 Sep 2012 10:06:56 -0400 Subject: [PATCH 05/65] Use Commit#show instead of Commits#show to view a single commit Commits#show (plural) is going to be for showing commit history on a specific path. --- app/controllers/commits_controller.rb | 32 +++++++++---------- app/roles/static_model.rb | 4 +++ app/views/{commits => commit}/show.html.haml | 0 app/views/commits/_commit.html.haml | 7 ++-- app/views/events/_commit.html.haml | 2 +- app/views/refs/blame.html.haml | 4 +-- app/views/repositories/_branch.html.haml | 2 +- lib/gitlab/markdown.rb | 2 +- .../requests/gitlab_flavored_markdown_spec.rb | 6 ++-- spec/routing/project_routing_spec.rb | 9 ++++++ 10 files changed, 40 insertions(+), 28 deletions(-) rename app/views/{commits => commit}/show.html.haml (100%) diff --git a/app/controllers/commits_controller.rb b/app/controllers/commits_controller.rb index 1e7aec00..83404bdc 100644 --- a/app/controllers/commits_controller.rb +++ b/app/controllers/commits_controller.rb @@ -26,25 +26,25 @@ class CommitsController < ApplicationController end end - def show - result = CommitLoad.new(project, current_user, params).execute + # def show + # result = CommitLoad.new(project, current_user, params).execute - @commit = result[:commit] + # @commit = result[:commit] - if @commit - @suppress_diff = result[:suppress_diff] - @note = result[:note] - @line_notes = result[:line_notes] - @notes_count = result[:notes_count] - @comments_allowed = true - else - return git_not_found! - end + # if @commit + # @suppress_diff = result[:suppress_diff] + # @note = result[:note] + # @line_notes = result[:line_notes] + # @notes_count = result[:notes_count] + # @comments_allowed = true + # else + # return git_not_found! + # end - if result[:status] == :huge_commit - render "huge_commit" and return - end - end + # if result[:status] == :huge_commit + # render "huge_commit" and return + # end + # end def compare result = Commit.compare(project, params[:from], params[:to]) diff --git a/app/roles/static_model.rb b/app/roles/static_model.rb index d26c8f47..d67af243 100644 --- a/app/roles/static_model.rb +++ b/app/roles/static_model.rb @@ -25,6 +25,10 @@ module StaticModel id end + def new_record? + false + end + def persisted? false end diff --git a/app/views/commits/show.html.haml b/app/views/commit/show.html.haml similarity index 100% rename from app/views/commits/show.html.haml rename to app/views/commit/show.html.haml diff --git a/app/views/commits/_commit.html.haml b/app/views/commits/_commit.html.haml index 61371d14..6abea76c 100644 --- a/app/views/commits/_commit.html.haml +++ b/app/views/commits/_commit.html.haml @@ -1,16 +1,15 @@ %li.commit .browse_code_link_holder %p - %strong= link_to "Browse Code »", tree_project_ref_path(@project, commit.id), class: "right" + %strong= link_to "Browse Code »", tree_project_ref_path(@project, commit), class: "right" %p - = link_to commit.short_id(8), project_commit_path(@project, id: commit.id), class: "commit_short_id" + = link_to commit.short_id(8), project_commit_path(@project, commit), class: "commit_short_id" %strong.commit-author-name= commit.author_name %span.dash – = image_tag gravatar_icon(commit.author_email), class: "avatar", width: 16 - = link_to_gfm truncate(commit.title, length: 50), project_commit_path(@project, id: commit.id), class: "row_title" + = link_to_gfm truncate(commit.title, length: 50), project_commit_path(@project, commit.id), class: "row_title" %span.committed_ago = time_ago_in_words(commit.committed_date) ago   - diff --git a/app/views/events/_commit.html.haml b/app/views/events/_commit.html.haml index ed4f33c0..ea417aa9 100644 --- a/app/views/events/_commit.html.haml +++ b/app/views/events/_commit.html.haml @@ -1,7 +1,7 @@ - commit = CommitDecorator.decorate(commit) %li.commit %p - = link_to commit.short_id(8), project_commit_path(project, id: commit.id), class: "commit_short_id" + = link_to commit.short_id(8), project_commit_path(project, commit), class: "commit_short_id" %span= commit.author_name – = image_tag gravatar_icon(commit.author_email), class: "avatar", width: 16 diff --git a/app/views/refs/blame.html.haml b/app/views/refs/blame.html.haml index eb66f597..ba0fad52 100644 --- a/app/views/refs/blame.html.haml +++ b/app/views/refs/blame.html.haml @@ -32,8 +32,8 @@ = commit.author_name %td.blame_commit   - %code= link_to commit.short_id, project_commit_path(@project, id: commit.id) - = link_to_gfm truncate(commit.title, length: 30), project_commit_path(@project, id: commit.id), class: "row_title" rescue "--broken encoding" + %code= link_to commit.short_id, project_commit_path(@project, commit) + = link_to_gfm truncate(commit.title, length: 30), project_commit_path(@project, commit), class: "row_title" rescue "--broken encoding" %td.lines = preserve do %pre diff --git a/app/views/repositories/_branch.html.haml b/app/views/repositories/_branch.html.haml index 64a633be..80028a82 100644 --- a/app/views/repositories/_branch.html.haml +++ b/app/views/repositories/_branch.html.haml @@ -7,7 +7,7 @@ - if branch.name == @project.root_ref %span.label default %td - = link_to project_commit_path(@project, id: commit.id) do + = link_to project_commit_path(@project, commit) do %code= commit.short_id = image_tag gravatar_icon(commit.author_email), class: "", width: 16 diff --git a/lib/gitlab/markdown.rb b/lib/gitlab/markdown.rb index 9201003e..9eb35b84 100644 --- a/lib/gitlab/markdown.rb +++ b/lib/gitlab/markdown.rb @@ -174,7 +174,7 @@ module Gitlab def reference_commit(identifier) if commit = @project.commit(identifier) - link_to(identifier, project_commit_path(@project, id: commit.id), html_options.merge(title: CommitDecorator.new(commit).link_title, class: "gfm gfm-commit #{html_options[:class]}")) + link_to(identifier, project_commit_path(@project, commit), html_options.merge(title: CommitDecorator.new(commit).link_title, class: "gfm gfm-commit #{html_options[:class]}")) end end end diff --git a/spec/requests/gitlab_flavored_markdown_spec.rb b/spec/requests/gitlab_flavored_markdown_spec.rb index 68d354b7..807d17d7 100644 --- a/spec/requests/gitlab_flavored_markdown_spec.rb +++ b/spec/requests/gitlab_flavored_markdown_spec.rb @@ -49,13 +49,13 @@ describe "Gitlab Flavored Markdown" do end it "should render title in commits#show" do - visit project_commit_path(project, id: commit.id) + visit project_commit_path(project, commit) page.should have_link("##{issue.id}") end it "should render description in commits#show" do - visit project_commit_path(project, id: commit.id) + visit project_commit_path(project, commit) page.should have_link("@#{fred.name}") end @@ -175,7 +175,7 @@ describe "Gitlab Flavored Markdown" do describe "for notes" do it "should render in commits#show", js: true do - visit project_commit_path(project, id: commit.id) + visit project_commit_path(project, commit) fill_in "note_note", with: "see ##{issue.id}" click_button "Add Comment" diff --git a/spec/routing/project_routing_spec.rb b/spec/routing/project_routing_spec.rb index d4ee5c33..9900a31f 100644 --- a/spec/routing/project_routing_spec.rb +++ b/spec/routing/project_routing_spec.rb @@ -298,6 +298,14 @@ describe HooksController, "routing" do end end +# project_commit GET /:project_id/commit/:id(.:format) commit#show {:id=>/[[:alnum:]]{6,40}/, :project_id=>/[^\/]+/} +describe CommitController, "routing" do + it "to #show" do + get("/gitlabhq/commit/4246fb").should route_to('commit#show', project_id: 'gitlabhq', id: '4246fb') + get("/gitlabhq/commit/4246fbd13872934f72a8fd0d6fb1317b47b59cb5").should route_to('commit#show', project_id: 'gitlabhq', id: '4246fbd13872934f72a8fd0d6fb1317b47b59cb5') + end +end + # compare_project_commits GET /:project_id/commits/compare(.:format) commits#compare # patch_project_commit GET /:project_id/commits/:id/patch(.:format) commits#patch # project_commits GET /:project_id/commits(.:format) commits#index @@ -317,6 +325,7 @@ describe CommitsController, "routing" do end it_behaves_like "RESTful project resources" do + let(:actions) { [:index, :show] } let(:controller) { 'commits' } end end From 567767bcf2af2c330c46fde820101fcf847e852f Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Mon, 17 Sep 2012 10:44:36 -0400 Subject: [PATCH 06/65] Add ref_extractor helper module for upcoming controllers --- lib/ref_extractor.rb | 64 ++++++++++++++++++++++++++++++++++ spec/lib/ref_extractor_spec.rb | 39 +++++++++++++++++++++ 2 files changed, 103 insertions(+) create mode 100644 lib/ref_extractor.rb create mode 100644 spec/lib/ref_extractor_spec.rb diff --git a/lib/ref_extractor.rb b/lib/ref_extractor.rb new file mode 100644 index 00000000..d3f02d6e --- /dev/null +++ b/lib/ref_extractor.rb @@ -0,0 +1,64 @@ +# Module providing an extract_ref method for controllers working with Git +# tree-ish + path params +# +# Given a string containing both a Git ref - such as a branch or tag - and a +# filesystem path joined by forward slashes, attempts to separate the two. +# +# Expects a @project instance variable to contain the active project. Used to +# check the input against a list of valid repository refs. +# +# Examples +# +# # No @project available +# extract_ref('master') +# # => ['', ''] +# +# extract_ref('master') +# # => ['master', '/'] +# +# extract_ref("f4b14494ef6abf3d144c28e4af0c20143383e062/CHANGELOG") +# # => ['f4b14494ef6abf3d144c28e4af0c20143383e062', '/CHANGELOG'] +# +# extract_ref("v2.0.0/README.md") +# # => ['v2.0.0', '/README.md'] +# +# extract_ref('issues/1234/app/models/project.rb') +# # => ['issues/1234', '/app/models/project.rb'] +# +# # Given an invalid branch, we fall back to just splitting on the first slash +# extract_ref('non/existent/branch/README.md') +# # => ['non', '/existent/branch/README.md'] +# +# Returns an Array where the first value is the tree-ish and the second is the +# path +module RefExtractor + def extract_ref(input) + pair = ['', ''] + + return pair unless @project + + if input.match(/^([[:alnum:]]{40})(.+)/) + # If the ref appears to be a SHA, we're done, just split the string + pair = $~.captures + else + # Append a trailing slash if we only get a ref and no file path + id = input + id += '/' unless id.include?('/') + + # Otherwise, attempt to detect the ref using a list of the project's + # branches and tags + valid_refs = @project.branches + @project.tags + valid_refs.select! { |v| id.start_with?("#{v}/") } + + if valid_refs.length != 1 + # No exact ref match, so just try our best + pair = id.match(/([^\/]+)(.+)/).captures + else + # Partition the string into the ref and the path, ignoring the empty first value + pair = id.partition(valid_refs.first)[1..-1] + end + end + + pair + end +end diff --git a/spec/lib/ref_extractor_spec.rb b/spec/lib/ref_extractor_spec.rb new file mode 100644 index 00000000..84347977 --- /dev/null +++ b/spec/lib/ref_extractor_spec.rb @@ -0,0 +1,39 @@ +require 'spec_helper' + +describe RefExtractor do + include RefExtractor + + let(:project) { double('project') } + + before do + @project = project + project.stub(:branches).and_return(['master', 'foo/bar/baz']) + project.stub(:tags).and_return(['v1.0.0', 'v2.0.0']) + end + + it "extracts a ref without a path" do + extract_ref('master').should == ['master', '/'] + end + + it "extracts a valid branch ref" do + extract_ref('foo/bar/baz/CHANGELOG').should == ['foo/bar/baz', '/CHANGELOG'] + end + + it "extracts a valid tag ref" do + extract_ref('v2.0.0/CHANGELOG').should == ['v2.0.0', '/CHANGELOG'] + end + + it "extracts a valid commit ref" do + extract_ref('f4b14494ef6abf3d144c28e4af0c20143383e062/CHANGELOG').should == + ['f4b14494ef6abf3d144c28e4af0c20143383e062', '/CHANGELOG'] + end + + it "falls back to a primitive split for an invalid ref" do + extract_ref('stable/CHANGELOG').should == ['stable', '/CHANGELOG'] + end + + it "returns an empty pair when no @project is set" do + @project = nil + extract_ref('master/CHANGELOG').should == ['', ''] + end +end From 6ddb35bd5e27829609e40b8dd87313a01fb6f293 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Mon, 17 Sep 2012 10:57:13 -0400 Subject: [PATCH 07/65] Move tree-related views from refs to trees --- app/views/tree/_head.html.haml | 11 +++++++++++ app/views/{refs => tree}/_submodule_item.html.haml | 0 app/views/{refs => tree}/_tree.html.haml | 0 app/views/{refs => tree}/_tree_commit.html.haml | 0 app/views/{refs => tree}/_tree_file.html.haml | 0 app/views/{refs => tree}/_tree_item.html.haml | 0 .../{refs/tree.html.haml => tree/show.html.haml} | 0 app/views/{refs/tree.js.haml => tree/show.js.haml} | 0 8 files changed, 11 insertions(+) create mode 100644 app/views/tree/_head.html.haml rename app/views/{refs => tree}/_submodule_item.html.haml (100%) rename app/views/{refs => tree}/_tree.html.haml (100%) rename app/views/{refs => tree}/_tree_commit.html.haml (100%) rename app/views/{refs => tree}/_tree_file.html.haml (100%) rename app/views/{refs => tree}/_tree_item.html.haml (100%) rename app/views/{refs/tree.html.haml => tree/show.html.haml} (100%) rename app/views/{refs/tree.js.haml => tree/show.js.haml} (100%) diff --git a/app/views/tree/_head.html.haml b/app/views/tree/_head.html.haml new file mode 100644 index 00000000..3592f573 --- /dev/null +++ b/app/views/tree/_head.html.haml @@ -0,0 +1,11 @@ +%ul.nav.nav-tabs + %li + = render partial: 'shared/ref_switcher', locals: {destination: 'tree', path: params[:path]} + %li{class: "#{'active' if (controller.controller_name == "refs") }"} + = link_to tree_project_ref_path(@project, @ref) do + Source + %li.right + .input-prepend.project_clone_holder + %button{class: "btn small active", :"data-clone" => @project.ssh_url_to_repo} SSH + %button{class: "btn small", :"data-clone" => @project.http_url_to_repo} HTTP + = text_field_tag :project_clone, @project.url_to_repo, class: "one_click_select span5" diff --git a/app/views/refs/_submodule_item.html.haml b/app/views/tree/_submodule_item.html.haml similarity index 100% rename from app/views/refs/_submodule_item.html.haml rename to app/views/tree/_submodule_item.html.haml diff --git a/app/views/refs/_tree.html.haml b/app/views/tree/_tree.html.haml similarity index 100% rename from app/views/refs/_tree.html.haml rename to app/views/tree/_tree.html.haml diff --git a/app/views/refs/_tree_commit.html.haml b/app/views/tree/_tree_commit.html.haml similarity index 100% rename from app/views/refs/_tree_commit.html.haml rename to app/views/tree/_tree_commit.html.haml diff --git a/app/views/refs/_tree_file.html.haml b/app/views/tree/_tree_file.html.haml similarity index 100% rename from app/views/refs/_tree_file.html.haml rename to app/views/tree/_tree_file.html.haml diff --git a/app/views/refs/_tree_item.html.haml b/app/views/tree/_tree_item.html.haml similarity index 100% rename from app/views/refs/_tree_item.html.haml rename to app/views/tree/_tree_item.html.haml diff --git a/app/views/refs/tree.html.haml b/app/views/tree/show.html.haml similarity index 100% rename from app/views/refs/tree.html.haml rename to app/views/tree/show.html.haml diff --git a/app/views/refs/tree.js.haml b/app/views/tree/show.js.haml similarity index 100% rename from app/views/refs/tree.js.haml rename to app/views/tree/show.js.haml From 2ddb19170624f81ec0cb3da26805a5515878aede Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Mon, 17 Sep 2012 11:10:04 -0400 Subject: [PATCH 08/65] Require 'github/markup' in Gemfile --- Gemfile | 2 +- app/controllers/refs_controller.rb | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/Gemfile b/Gemfile index 71eafaba..8e2bdb9c 100644 --- a/Gemfile +++ b/Gemfile @@ -57,7 +57,7 @@ gem "seed-fu" # Markdown to HTML gem "redcarpet", "~> 2.1.1" -gem "github-markup", "~> 0.7.4" +gem "github-markup", "~> 0.7.4", require: 'github/markup' # Servers gem "thin" diff --git a/app/controllers/refs_controller.rb b/app/controllers/refs_controller.rb index 90361437..3f81a2ca 100644 --- a/app/controllers/refs_controller.rb +++ b/app/controllers/refs_controller.rb @@ -1,5 +1,3 @@ -require 'github/markup' - class RefsController < ApplicationController include Gitlab::Encode before_filter :project From 884eb732977ee785c48a954542e8735cd572e1fe Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Mon, 17 Sep 2012 12:26:29 -0400 Subject: [PATCH 09/65] Enable tree resource, remove old tree routes --- config/routes.rb | 16 +++------------- spec/routing/project_routing_spec.rb | 21 +++++++-------------- 2 files changed, 10 insertions(+), 27 deletions(-) diff --git a/config/routes.rb b/config/routes.rb index 4f55d7e5..d1ed8749 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -122,24 +122,14 @@ Gitlab::Application.routes.draw do end member do - get "tree", constraints: { id: /[a-zA-Z.\/0-9_\-]+/ } - get "logs_tree", constraints: { id: /[a-zA-Z.\/0-9_\-]+/ } - get "blob", constraints: { id: /[a-zA-Z.0-9\/_\-]+/, path: /.*/ } - # tree viewer - get "tree/:path" => "refs#tree", - as: :tree_file, - constraints: { - id: /[a-zA-Z.0-9\/_\-]+/, - path: /.*/ - } - - # tree viewer + # tree viewer logs + get "logs_tree", constraints: { id: /[a-zA-Z.\/0-9_\-]+/ } get "logs_tree/:path" => "refs#logs_tree", as: :logs_file, constraints: { @@ -217,7 +207,7 @@ Gitlab::Application.routes.draw do # resources :blame, only: [:show], constraints: {id: /.+/} # resources :blob, only: [:show], constraints: {id: /.+/} # resources :raw, only: [:show], constraints: {id: /.+/} - # resources :tree, only: [:show], constraints: {id: /.+/} + resources :tree, only: [:show], constraints: {id: /.+/} end root to: "dashboard#index" diff --git a/spec/routing/project_routing_spec.rb b/spec/routing/project_routing_spec.rb index 9900a31f..f9c99a20 100644 --- a/spec/routing/project_routing_spec.rb +++ b/spec/routing/project_routing_spec.rb @@ -192,10 +192,8 @@ describe ProtectedBranchesController, "routing" do end # switch_project_refs GET /:project_id/switch(.:format) refs#switch -# tree_project_ref GET /:project_id/:id/tree(.:format) refs#tree -# logs_tree_project_ref GET /:project_id/:id/logs_tree(.:format) refs#logs_tree # blob_project_ref GET /:project_id/:id/blob(.:format) refs#blob -# tree_file_project_ref GET /:project_id/:id/tree/:path(.:format) refs#tree +# logs_tree_project_ref GET /:project_id/:id/logs_tree(.:format) refs#logs_tree # logs_file_project_ref GET /:project_id/:id/logs_tree/:path(.:format) refs#logs_tree # blame_file_project_ref GET /:project_id/:id/blame/:path(.:format) refs#blame describe RefsController, "routing" do @@ -203,11 +201,6 @@ describe RefsController, "routing" do get("/gitlabhq/switch").should route_to('refs#switch', project_id: 'gitlabhq') end - it "to #tree" do - get("/gitlabhq/stable/tree").should route_to('refs#tree', project_id: 'gitlabhq', id: 'stable') - get("/gitlabhq/stable/tree/foo/bar/baz").should route_to('refs#tree', project_id: 'gitlabhq', id: 'stable', path: 'foo/bar/baz') - end - it "to #logs_tree" do get("/gitlabhq/stable/logs_tree").should route_to('refs#logs_tree', project_id: 'gitlabhq', id: 'stable') get("/gitlabhq/stable/logs_tree/foo/bar/baz").should route_to('refs#logs_tree', project_id: 'gitlabhq', id: 'stable', path: 'foo/bar/baz') @@ -406,6 +399,12 @@ describe NotesController, "routing" do end end +describe TreeController, "routing" do + it "to #show" do + get("/gitlabhq/tree/master/app/models/project.rb").should route_to('tree#show', project_id: 'gitlabhq', id: 'master/app/models/project.rb') + end +end + # TODO: Pending # # /:project_id/blame/*path @@ -457,10 +456,4 @@ describe "pending routing" do get("/gitlabhq/raw/master/app/models/project.rb").should route_to('raw#show', project_id: 'gitlabhq', id: 'master/app/models/project.rb') end end - - describe "/:project_id/tree/:id" do - it "routes to a ref with a path" do - get("/gitlabhq/tree/master/app/models/project.rb").should route_to('tree#show', project_id: 'gitlabhq', id: 'master/app/models/project.rb') - end - end end From e33cbb9b4252e2617bcb4c3f850c47aae43e4d83 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Mon, 17 Sep 2012 12:38:59 -0400 Subject: [PATCH 10/65] Add TreeController and spec --- app/controllers/tree_controller.rb | 49 ++++++++++++++++++++++++ app/models/tree.rb | 10 +++-- spec/controllers/tree_controller_spec.rb | 43 +++++++++++++++++++++ spec/spec_helper.rb | 1 + 4 files changed, 100 insertions(+), 3 deletions(-) create mode 100644 app/controllers/tree_controller.rb create mode 100644 spec/controllers/tree_controller_spec.rb diff --git a/app/controllers/tree_controller.rb b/app/controllers/tree_controller.rb new file mode 100644 index 00000000..15bbb1a3 --- /dev/null +++ b/app/controllers/tree_controller.rb @@ -0,0 +1,49 @@ +# Controller for viewing a repository's file structure +class TreeController < ApplicationController + # Thrown when given an invalid path + class InvalidPathError < StandardError; end + + include RefExtractor + + layout "project" + + before_filter :project + + # Authorize + before_filter :add_project_abilities + before_filter :authorize_read_project! + before_filter :authorize_code_access! + before_filter :require_non_empty_project + + before_filter :define_tree_vars + + def show + respond_to do |format| + format.html + # Disable cache so browser history works + format.js { no_cache_headers } + end + end + + private + + def define_tree_vars + @ref, @path = extract_ref(params[:id]) + + @id = File.join(@ref, @path) + @repo = @project.repo + @commit = CommitDecorator.decorate(@project.commit(@ref)) + + @tree = Tree.new(@commit.tree, @project, @ref, @path.gsub(/^\//, '')) + @tree = TreeDecorator.new(@tree) + + raise InvalidPathError if @tree.invalid? + + @hex_path = Digest::SHA1.hexdigest(@path) + + @history_path = project_tree_path(@project, @id) + @logs_path = logs_file_project_ref_path(@project, @ref, @path) + rescue NoMethodError, InvalidPathError + not_found! + end +end diff --git a/app/models/tree.rb b/app/models/tree.rb index d65e50ab..88e8f2f4 100644 --- a/app/models/tree.rb +++ b/app/models/tree.rb @@ -1,5 +1,5 @@ class Tree - include Linguist::BlobHelper + include Linguist::BlobHelper attr_accessor :path, :tree, :project, :ref delegate :contents, @@ -14,8 +14,8 @@ class Tree to: :tree def initialize(raw_tree, project, ref = nil, path = nil) - @project, @ref, @path = project, ref, path, - @tree = if path + @project, @ref, @path = project, ref, path + @tree = if path.present? raw_tree / path.dup.force_encoding('ascii-8bit') else raw_tree @@ -26,6 +26,10 @@ class Tree tree.is_a?(Grit::Blob) end + def invalid? + tree.nil? + end + def empty? data.blank? end diff --git a/spec/controllers/tree_controller_spec.rb b/spec/controllers/tree_controller_spec.rb new file mode 100644 index 00000000..b9295537 --- /dev/null +++ b/spec/controllers/tree_controller_spec.rb @@ -0,0 +1,43 @@ +require 'spec_helper' + +describe TreeController do + let(:project) { create(:project) } + let(:user) { create(:user) } + + before do + sign_in(user) + + project.add_access(user, :read, :admin) + + project.stub(:branches).and_return(['master', 'foo/bar/baz']) + project.stub(:tags).and_return(['v1.0.0', 'v2.0.0']) + controller.instance_variable_set(:@project, project) + end + + describe "GET show" do + # Make sure any errors accessing the tree in our views bubble up to this spec + render_views + + before { get :show, project_id: project.code, id: id } + + context "valid branch, no path" do + let(:id) { 'master' } + it { should respond_with(:success) } + end + + context "valid branch, valid path" do + let(:id) { 'master/README.md' } + it { should respond_with(:success) } + end + + context "valid branch, invalid path" do + let(:id) { 'master/invalid-path.rb' } + it { should respond_with(:not_found) } + end + + context "invalid branch, valid path" do + let(:id) { 'invalid-branch/README.md' } + it { should respond_with(:not_found) } + end + end +end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index d381b3f1..4700c3fe 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -28,6 +28,7 @@ RSpec.configure do |config| config.include LoginHelpers, type: :request config.include GitoliteStub config.include FactoryGirl::Syntax::Methods + config.include Devise::TestHelpers, type: :controller # If you're not using ActiveRecord, or you'd prefer not to run each of your # examples within a transaction, remove the following line or assign false From 79a02df92e18137787371468be5c8897ad2f97b0 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Mon, 17 Sep 2012 12:39:57 -0400 Subject: [PATCH 11/65] Update usages of tree_file_project_ref_path to project_tree_path --- app/controllers/refs_controller.rb | 6 +++--- app/decorators/tree_decorator.rb | 4 ++-- app/helpers/application_helper.rb | 10 +++++----- app/helpers/tree_helper.rb | 5 +++++ app/views/commits/_commit.html.haml | 2 +- app/views/commits/_commit_box.html.haml | 2 +- app/views/commits/_diffs.html.haml | 2 +- app/views/layouts/_head.html.haml | 2 +- app/views/layouts/_project_menu.html.haml | 2 +- app/views/refs/_head.html.haml | 2 +- app/views/refs/blame.html.haml | 4 ++-- app/views/refs/logs_tree.js.haml | 2 +- app/views/tree/_head.html.haml | 4 ++-- app/views/tree/_tree.html.haml | 10 +++++----- app/views/tree/_tree_file.html.haml | 4 ++-- app/views/tree/_tree_item.html.haml | 4 ++-- features/steps/project/project_browse_files.rb | 2 +- features/steps/shared/paths.rb | 6 +++--- spec/requests/gitlab_flavored_markdown_spec.rb | 2 +- spec/requests/security/project_access_spec.rb | 2 +- 20 files changed, 41 insertions(+), 36 deletions(-) diff --git a/app/controllers/refs_controller.rb b/app/controllers/refs_controller.rb index 3f81a2ca..6cdd0953 100644 --- a/app/controllers/refs_controller.rb +++ b/app/controllers/refs_controller.rb @@ -18,7 +18,7 @@ class RefsController < ApplicationController respond_to do |format| format.html do new_path = if params[:destination] == "tree" - tree_project_ref_path(@project, params[:ref]) + project_tree_path(@project, params[:ref]) else project_commits_path(@project, ref: params[:ref]) end @@ -96,10 +96,10 @@ class RefsController < ApplicationController @hex_path = Digest::SHA1.hexdigest(params[:path] || "/") if params[:path] - @history_path = tree_file_project_ref_path(@project, @ref, params[:path]) + @history_path = project_tree_path(@project, File.join(@ref, params[:path])) @logs_path = logs_file_project_ref_path(@project, @ref, params[:path]) else - @history_path = tree_project_ref_path(@project, @ref) + @history_path = project_tree_path(@project, @ref) @logs_path = logs_tree_project_ref_path(@project, @ref) end rescue diff --git a/app/decorators/tree_decorator.rb b/app/decorators/tree_decorator.rb index 80c48da7..05f029d0 100644 --- a/app/decorators/tree_decorator.rb +++ b/app/decorators/tree_decorator.rb @@ -15,7 +15,7 @@ class TreeDecorator < ApplicationDecorator part_path = part if part_path.empty? next unless parts.last(2).include?(part) if parts.count > max_links - yield(h.link_to(h.truncate(part, length: 40), h.tree_file_project_ref_path(project, ref, path: part_path), remote: :true)) + yield(h.link_to(h.truncate(part, length: 40), h.project_tree_path(project, h.tree_join(ref, part_path)), remote: :true)) end end end @@ -26,7 +26,7 @@ class TreeDecorator < ApplicationDecorator def up_dir_path file = File.join(path, "..") - h.tree_file_project_ref_path(project, ref, file) + h.project_tree_path(project, h.tree_join(ref, file)) end def history_path diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 0938dc23..d90fb32f 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -58,11 +58,11 @@ module ApplicationHelper if @project && !@project.new_record? project_nav = [ - { label: "#{@project.name} / Issues", url: project_issues_path(@project) }, - { label: "#{@project.name} / Wall", url: wall_project_path(@project) }, - { label: "#{@project.name} / Tree", url: tree_project_ref_path(@project, @project.root_ref) }, - { label: "#{@project.name} / Commits", url: project_commits_path(@project) }, - { label: "#{@project.name} / Team", url: project_team_index_path(@project) } + { label: "#{@project.name} / Issues", url: project_issues_path(@project) }, + { label: "#{@project.name} / Wall", url: wall_project_path(@project) }, + { label: "#{@project.name} / Tree", url: project_tree_path(@project, @ref || @project.root_ref) }, + { label: "#{@project.name} / Commits", url: project_commits_path(@project, @ref || @project.root_ref) }, + { label: "#{@project.name} / Team", url: project_team_index_path(@project) } ] end diff --git a/app/helpers/tree_helper.rb b/app/helpers/tree_helper.rb index 2b7265ca..81a16989 100644 --- a/app/helpers/tree_helper.rb +++ b/app/helpers/tree_helper.rb @@ -39,4 +39,9 @@ module TreeHelper def gitlab_markdown?(filename) filename.end_with?(*%w(.mdown .md .markdown)) end + + # Simple shortcut to File.join + def tree_join(*args) + File.join(*args) + end end diff --git a/app/views/commits/_commit.html.haml b/app/views/commits/_commit.html.haml index 6abea76c..259e8e26 100644 --- a/app/views/commits/_commit.html.haml +++ b/app/views/commits/_commit.html.haml @@ -1,7 +1,7 @@ %li.commit .browse_code_link_holder %p - %strong= link_to "Browse Code »", tree_project_ref_path(@project, commit), class: "right" + %strong= link_to "Browse Code »", project_tree_path(@project, commit), class: "right" %p = link_to commit.short_id(8), project_commit_path(@project, commit), class: "commit_short_id" %strong.commit-author-name= commit.author_name diff --git a/app/views/commits/_commit_box.html.haml b/app/views/commits/_commit_box.html.haml index 572337de..3daf63f4 100644 --- a/app/views/commits/_commit_box.html.haml +++ b/app/views/commits/_commit_box.html.haml @@ -8,7 +8,7 @@ = link_to patch_project_commit_path(@project, @commit.id), class: "btn small grouped" do %i.icon-download-alt Get Patch - = link_to tree_project_ref_path(@project, @commit.id), class: "browse-button primary grouped" do + = link_to project_tree_path(@project, @commit), class: "browse-button primary grouped" do %strong Browse Code » %h3.commit-title.page_title = gfm escape_once(@commit.title) diff --git a/app/views/commits/_diffs.html.haml b/app/views/commits/_diffs.html.haml index b590d64c..bc53911c 100644 --- a/app/views/commits/_diffs.html.haml +++ b/app/views/commits/_diffs.html.haml @@ -24,7 +24,7 @@ %i.icon-file %span{id: "#{diff.old_path}"}= diff.old_path - else - = link_to tree_file_project_ref_path(@project, @commit.id, diff.new_path) do + = link_to project_tree_path(@project, @commit, diff.new_path) do %i.icon-file %span{id: "#{diff.new_path}"}= diff.new_path %br/ diff --git a/app/views/layouts/_head.html.haml b/app/views/layouts/_head.html.haml index c076a3a1..06da82d3 100644 --- a/app/views/layouts/_head.html.haml +++ b/app/views/layouts/_head.html.haml @@ -10,7 +10,7 @@ - if controller_name == 'projects' && action_name == 'index' = auto_discovery_link_tag :atom, projects_url(:atom, private_token: current_user.private_token), title: "Dashboard feed" - if @project && !@project.new_record? - - if current_page?(tree_project_ref_path(@project, @project.root_ref)) || current_page?(project_commits_path(@project)) + - if current_page?(project_tree_path(@project, @project.root_ref)) || current_page?(project_commits_path(@project)) = auto_discovery_link_tag(:atom, project_commits_url(@project, :atom, ref: @ref, private_token: current_user.private_token), title: "Recent commits to #{@project.name}:#{@ref}") - if request.path == project_issues_path(@project) = auto_discovery_link_tag(:atom, project_issues_url(@project, :atom, private_token: current_user.private_token), title: "#{@project.name} issues") diff --git a/app/views/layouts/_project_menu.html.haml b/app/views/layouts/_project_menu.html.haml index 04eaec5a..11e632b9 100644 --- a/app/views/layouts/_project_menu.html.haml +++ b/app/views/layouts/_project_menu.html.haml @@ -5,7 +5,7 @@ - if @project.repo_exists? - if can? current_user, :download_code, @project %li{class: tree_tab_class} - = link_to tree_project_ref_path(@project, @project.root_ref) do + = link_to project_tree_path(@project, @project.root_ref) do Files %li{class: commit_tab_class} = link_to "Commits", project_commits_path(@project) diff --git a/app/views/refs/_head.html.haml b/app/views/refs/_head.html.haml index 3592f573..2a6052a1 100644 --- a/app/views/refs/_head.html.haml +++ b/app/views/refs/_head.html.haml @@ -2,7 +2,7 @@ %li = render partial: 'shared/ref_switcher', locals: {destination: 'tree', path: params[:path]} %li{class: "#{'active' if (controller.controller_name == "refs") }"} - = link_to tree_project_ref_path(@project, @ref) do + = link_to project_tree_path(@project, @ref) do Source %li.right .input-prepend.project_clone_holder diff --git a/app/views/refs/blame.html.haml b/app/views/refs/blame.html.haml index ba0fad52..75b86315 100644 --- a/app/views/refs/blame.html.haml +++ b/app/views/refs/blame.html.haml @@ -4,7 +4,7 @@ %ul.breadcrumb %li %span.arrow - = link_to tree_project_ref_path(@project, @ref, path: nil) do + = link_to project_tree_path(@project, @ref) do = @project.name - @tree.breadcrumbs(6) do |link| \/ @@ -20,7 +20,7 @@ %span.options = link_to "raw", blob_project_ref_path(@project, @ref, path: params[:path]), class: "btn very_small", target: "_blank" = link_to "history", project_commits_path(@project, path: params[:path], ref: @ref), class: "btn very_small" - = link_to "source", tree_file_project_ref_path(@project, @ref, path: params[:path]), class: "btn very_small" + = link_to "source", project_tree_path(@project, tree_join(@ref, params[:path])), class: "btn very_small" .file_content.blame %table - @blame.each do |commit, lines| diff --git a/app/views/refs/logs_tree.js.haml b/app/views/refs/logs_tree.js.haml index 61ccbaee..b0ac0c4b 100644 --- a/app/views/refs/logs_tree.js.haml +++ b/app/views/refs/logs_tree.js.haml @@ -6,4 +6,4 @@ :plain var row = $("table.table_#{@hex_path} tr.file_#{hexdigest(file_name)}"); row.find("td.tree_time_ago").html('#{escape_javascript(time_ago_in_words(content_commit.committed_date))} ago'); - row.find("td.tree_commit").html('#{escape_javascript(render("tree_commit", tm: tm, content_commit: content_commit))}'); + row.find("td.tree_commit").html('#{escape_javascript(render("tree/tree_commit", tm: tm, content_commit: content_commit))}'); diff --git a/app/views/tree/_head.html.haml b/app/views/tree/_head.html.haml index 3592f573..5126891e 100644 --- a/app/views/tree/_head.html.haml +++ b/app/views/tree/_head.html.haml @@ -1,8 +1,8 @@ %ul.nav.nav-tabs %li = render partial: 'shared/ref_switcher', locals: {destination: 'tree', path: params[:path]} - %li{class: "#{'active' if (controller.controller_name == "refs") }"} - = link_to tree_project_ref_path(@project, @ref) do + %li{class: "#{'active' if (controller.controller_name == "tree") }"} + = link_to project_tree_path(@project, @ref) do Source %li.right .input-prepend.project_clone_holder diff --git a/app/views/tree/_tree.html.haml b/app/views/tree/_tree.html.haml index 55078718..439b4ec2 100644 --- a/app/views/tree/_tree.html.haml +++ b/app/views/tree/_tree.html.haml @@ -1,7 +1,7 @@ %ul.breadcrumb %li %span.arrow - = link_to tree_project_ref_path(@project, @ref, path: nil), remote: true do + = link_to project_tree_path(@project, @ref), remote: true do = @project.name - tree.breadcrumbs(6) do |link| \/ @@ -10,7 +10,7 @@ %div.tree_progress #tree-content-holder - if tree.is_blob? - = render partial: "refs/tree_file", locals: { name: tree.name, content: tree.data, file: tree } + = render partial: "tree/tree_file", locals: { name: tree.name, content: tree.data, file: tree } - else - contents = tree.contents %table#tree-slider{class: "table_#{@hex_path}" } @@ -31,11 +31,11 @@ - index = 0 - contents.select{ |i| i.is_a?(Grit::Tree)}.each do |content| - = render partial: "refs/tree_item", locals: { content: content, index: (index += 1) } + = render partial: "tree/tree_item", locals: { content: content, index: (index += 1) } - contents.select{ |i| i.is_a?(Grit::Blob)}.each do |content| - = render partial: "refs/tree_item", locals: { content: content, index: (index += 1) } + = render partial: "tree/tree_item", locals: { content: content, index: (index += 1) } - contents.select{ |i| i.is_a?(Grit::Submodule)}.each do |content| - = render partial: "refs/submodule_item", locals: { content: content, index: (index += 1) } + = render partial: "tree/submodule_item", locals: { content: content, index: (index += 1) } - if content = contents.select{ |c| c.is_a?(Grit::Blob) and c.name =~ /^readme/i }.first .file_holder#README diff --git a/app/views/tree/_tree_file.html.haml b/app/views/tree/_tree_file.html.haml index 76173e24..ae8f1ccf 100644 --- a/app/views/tree/_tree_file.html.haml +++ b/app/views/tree/_tree_file.html.haml @@ -5,9 +5,9 @@ = name.force_encoding('utf-8') %small #{file.mode} %span.options - = link_to "raw", blob_project_ref_path(@project, @ref, path: params[:path]), class: "btn very_small", target: "_blank" + = link_to "raw", blob_project_ref_path(@project, @ref, path: @path), class: "btn very_small", target: "_blank" = link_to "history", project_commits_path(@project, path: params[:path], ref: @ref), class: "btn very_small" - = link_to "blame", blame_file_project_ref_path(@project, @ref, path: params[:path]), class: "btn very_small" + = link_to "blame", blame_file_project_ref_path(@project, @ref, path: @path.gsub(/^\//, '')), class: "btn very_small" - if file.text? - if gitlab_markdown?(name) .file_content.wiki diff --git a/app/views/tree/_tree_item.html.haml b/app/views/tree/_tree_item.html.haml index d4c4ee8d..226c380f 100644 --- a/app/views/tree/_tree_item.html.haml +++ b/app/views/tree/_tree_item.html.haml @@ -1,8 +1,8 @@ - file = tree_full_path(content) -%tr{ class: "tree-item #{tree_hex_class(content)}", url: tree_file_project_ref_path(@project, @ref, file) } +%tr{ class: "tree-item #{tree_hex_class(content)}", url: project_tree_path(@project, tree_join(@id, file)) } %td.tree-item-file-name = tree_icon(content) - %strong= link_to truncate(content.name, length: 40), tree_file_project_ref_path(@project, @ref || @commit.id, file), remote: :true + %strong= link_to truncate(content.name, length: 40), project_tree_path(@project, tree_join(@id || @commit.id, file)), remote: :true %td.tree_time_ago.cgray - if index == 1 %span.log_loading diff --git a/features/steps/project/project_browse_files.rb b/features/steps/project/project_browse_files.rb index 67c553ce..652daba0 100644 --- a/features/steps/project/project_browse_files.rb +++ b/features/steps/project/project_browse_files.rb @@ -10,7 +10,7 @@ class ProjectBrowseFiles < Spinach::FeatureSteps end Then 'I should see files from repository for "8470d70"' do - current_path.should == tree_project_ref_path(@project, "8470d70") + current_path.should == project_tree_path(@project, "8470d70") page.should have_content "app" page.should have_content "History" page.should have_content "Gemfile" diff --git a/features/steps/shared/paths.rb b/features/steps/shared/paths.rb index 93ad0219..1839bc81 100644 --- a/features/steps/shared/paths.rb +++ b/features/steps/shared/paths.rb @@ -51,15 +51,15 @@ module SharedPaths end Given 'I visit project source page' do - visit tree_project_ref_path(@project, @project.root_ref) + visit project_tree_path(@project, @project.root_ref) end Given 'I visit blob file from repo' do - visit tree_project_ref_path(@project, ValidCommit::ID, :path => ValidCommit::BLOB_FILE_PATH) + visit project_tree_path(@project, File.join(ValidCommit::ID, ValidCommit::BLOB_FILE_PATH)) end Given 'I visit project source page for "8470d70"' do - visit tree_project_ref_path(@project, "8470d70") + visit project_tree_path(@project, "8470d70") end Given 'I visit project tags page' do diff --git a/spec/requests/gitlab_flavored_markdown_spec.rb b/spec/requests/gitlab_flavored_markdown_spec.rb index 807d17d7..db3f89cb 100644 --- a/spec/requests/gitlab_flavored_markdown_spec.rb +++ b/spec/requests/gitlab_flavored_markdown_spec.rb @@ -61,7 +61,7 @@ describe "Gitlab Flavored Markdown" do end it "should render title in refs#tree", js: true do - visit tree_project_ref_path(project, id: @branch_name) + visit project_tree_path(project, @branch_name) within(".tree_commit") do page.should have_link("##{issue.id}") diff --git a/spec/requests/security/project_access_spec.rb b/spec/requests/security/project_access_spec.rb index af0d5fcd..4b6abd3e 100644 --- a/spec/requests/security/project_access_spec.rb +++ b/spec/requests/security/project_access_spec.rb @@ -37,7 +37,7 @@ describe "Application access" do end describe "GET /project_code/master/tree" do - subject { tree_project_ref_path(@project, @project.root_ref) } + subject { project_tree_path(@project, @project.root_ref) } it { should be_allowed_for @u1 } it { should be_allowed_for @u3 } From 339dfa32b810b8f15c26c85e692a18a0f47b9fca Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Mon, 17 Sep 2012 13:07:17 -0400 Subject: [PATCH 12/65] Fix ref_switcher path --- app/views/shared/_ref_switcher.html.haml | 2 +- app/views/tree/_head.html.haml | 2 +- app/views/tree/show.js.haml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/views/shared/_ref_switcher.html.haml b/app/views/shared/_ref_switcher.html.haml index e0c89522..8b44cf19 100644 --- a/app/views/shared/_ref_switcher.html.haml +++ b/app/views/shared/_ref_switcher.html.haml @@ -1,5 +1,5 @@ = form_tag switch_project_refs_path(@project), method: :get, class: "project-refs-form" do = select_tag "ref", grouped_options_refs, class: "project-refs-select chosen" = hidden_field_tag :destination, destination - - if respond_to?(:path) + - if defined?(path) = hidden_field_tag :path, path diff --git a/app/views/tree/_head.html.haml b/app/views/tree/_head.html.haml index 5126891e..c7ebbf88 100644 --- a/app/views/tree/_head.html.haml +++ b/app/views/tree/_head.html.haml @@ -1,6 +1,6 @@ %ul.nav.nav-tabs %li - = render partial: 'shared/ref_switcher', locals: {destination: 'tree', path: params[:path]} + = render partial: 'shared/ref_switcher', locals: {destination: 'tree', path: @path} %li{class: "#{'active' if (controller.controller_name == "tree") }"} = link_to project_tree_path(@project, @ref) do Source diff --git a/app/views/tree/show.js.haml b/app/views/tree/show.js.haml index 92e90579..3d0215ff 100644 --- a/app/views/tree/show.js.haml +++ b/app/views/tree/show.js.haml @@ -2,7 +2,7 @@ // Load Files list $("#tree-holder").html("#{escape_javascript(render(partial: "tree", locals: {repo: @repo, commit: @commit, tree: @tree}))}"); $("#tree-content-holder").show("slide", { direction: "right" }, 150); - $('.project-refs-form #path').val("#{params[:path]}"); + $('.project-refs-form #path').val("#{@path}"); // Load last commit log for each file in tree $('#tree-slider').waitForImages(function() { From 94af622c879681683c84b6537c2adea52c78b2a2 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Mon, 17 Sep 2012 13:32:17 -0400 Subject: [PATCH 13/65] Move refs/blame view to blame/show --- app/views/{refs/blame.html.haml => blame/show.html.haml} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename app/views/{refs/blame.html.haml => blame/show.html.haml} (100%) diff --git a/app/views/refs/blame.html.haml b/app/views/blame/show.html.haml similarity index 100% rename from app/views/refs/blame.html.haml rename to app/views/blame/show.html.haml From 37f0b600bc9a994136791e6838b3228c56f909b2 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Mon, 17 Sep 2012 13:33:04 -0400 Subject: [PATCH 14/65] Another RefExtractor refactor --- lib/ref_extractor.rb | 74 ++++++++++++++++++---------------- spec/lib/ref_extractor_spec.rb | 57 +++++++++++++++++--------- 2 files changed, 78 insertions(+), 53 deletions(-) diff --git a/lib/ref_extractor.rb b/lib/ref_extractor.rb index d3f02d6e..d7d446b1 100644 --- a/lib/ref_extractor.rb +++ b/lib/ref_extractor.rb @@ -1,37 +1,39 @@ # Module providing an extract_ref method for controllers working with Git # tree-ish + path params -# -# Given a string containing both a Git ref - such as a branch or tag - and a -# filesystem path joined by forward slashes, attempts to separate the two. -# -# Expects a @project instance variable to contain the active project. Used to -# check the input against a list of valid repository refs. -# -# Examples -# -# # No @project available -# extract_ref('master') -# # => ['', ''] -# -# extract_ref('master') -# # => ['master', '/'] -# -# extract_ref("f4b14494ef6abf3d144c28e4af0c20143383e062/CHANGELOG") -# # => ['f4b14494ef6abf3d144c28e4af0c20143383e062', '/CHANGELOG'] -# -# extract_ref("v2.0.0/README.md") -# # => ['v2.0.0', '/README.md'] -# -# extract_ref('issues/1234/app/models/project.rb') -# # => ['issues/1234', '/app/models/project.rb'] -# -# # Given an invalid branch, we fall back to just splitting on the first slash -# extract_ref('non/existent/branch/README.md') -# # => ['non', '/existent/branch/README.md'] -# -# Returns an Array where the first value is the tree-ish and the second is the -# path module RefExtractor + # Thrown when given an invalid path + class InvalidPathError < StandardError; end + + # Given a string containing both a Git ref - such as a branch or tag - and a + # filesystem path joined by forward slashes, attempts to separate the two. + # + # Expects a @project instance variable to contain the active project. Used to + # check the input against a list of valid repository refs. + # + # Examples + # + # # No @project available + # extract_ref('master') + # # => ['', ''] + # + # extract_ref('master') + # # => ['master', ''] + # + # extract_ref("f4b14494ef6abf3d144c28e4af0c20143383e062/CHANGELOG") + # # => ['f4b14494ef6abf3d144c28e4af0c20143383e062', 'CHANGELOG'] + # + # extract_ref("v2.0.0/README.md") + # # => ['v2.0.0', 'README.md'] + # + # extract_ref('issues/1234/app/models/project.rb') + # # => ['issues/1234', 'app/models/project.rb'] + # + # # Given an invalid branch, we fall back to just splitting on the first slash + # extract_ref('non/existent/branch/README.md') + # # => ['non', 'existent/branch/README.md'] + # + # Returns an Array where the first value is the tree-ish and the second is the + # path def extract_ref(input) pair = ['', ''] @@ -41,24 +43,28 @@ module RefExtractor # If the ref appears to be a SHA, we're done, just split the string pair = $~.captures else + # Otherwise, attempt to detect the ref using a list of the project's + # branches and tags + # Append a trailing slash if we only get a ref and no file path id = input id += '/' unless id.include?('/') - # Otherwise, attempt to detect the ref using a list of the project's - # branches and tags valid_refs = @project.branches + @project.tags valid_refs.select! { |v| id.start_with?("#{v}/") } if valid_refs.length != 1 # No exact ref match, so just try our best - pair = id.match(/([^\/]+)(.+)/).captures + pair = id.match(/([^\/]+)(.*)/).captures else # Partition the string into the ref and the path, ignoring the empty first value pair = id.partition(valid_refs.first)[1..-1] end end + # Remove leading slash from path + pair[1].gsub!(/^\//, '') + pair end end diff --git a/spec/lib/ref_extractor_spec.rb b/spec/lib/ref_extractor_spec.rb index 84347977..1e3d178f 100644 --- a/spec/lib/ref_extractor_spec.rb +++ b/spec/lib/ref_extractor_spec.rb @@ -11,29 +11,48 @@ describe RefExtractor do project.stub(:tags).and_return(['v1.0.0', 'v2.0.0']) end - it "extracts a ref without a path" do - extract_ref('master').should == ['master', '/'] - end + describe '#extract_ref' do + it "returns an empty pair when no @project is set" do + @project = nil + extract_ref('master/CHANGELOG').should == ['', ''] + end - it "extracts a valid branch ref" do - extract_ref('foo/bar/baz/CHANGELOG').should == ['foo/bar/baz', '/CHANGELOG'] - end + context "without a path" do + it "extracts a valid branch" do + extract_ref('master').should == ['master', ''] + end - it "extracts a valid tag ref" do - extract_ref('v2.0.0/CHANGELOG').should == ['v2.0.0', '/CHANGELOG'] - end + it "extracts a valid tag" do + extract_ref('v2.0.0').should == ['v2.0.0', ''] + end - it "extracts a valid commit ref" do - extract_ref('f4b14494ef6abf3d144c28e4af0c20143383e062/CHANGELOG').should == - ['f4b14494ef6abf3d144c28e4af0c20143383e062', '/CHANGELOG'] - end + it "extracts a valid commit ref without a path" do + extract_ref('f4b14494ef6abf3d144c28e4af0c20143383e062').should == + ['f4b14494ef6abf3d144c28e4af0c20143383e062', ''] + end - it "falls back to a primitive split for an invalid ref" do - extract_ref('stable/CHANGELOG').should == ['stable', '/CHANGELOG'] - end + it "falls back to a primitive split for an invalid ref" do + extract_ref('stable').should == ['stable', ''] + end + end - it "returns an empty pair when no @project is set" do - @project = nil - extract_ref('master/CHANGELOG').should == ['', ''] + context "with a path" do + it "extracts a valid branch" do + extract_ref('foo/bar/baz/CHANGELOG').should == ['foo/bar/baz', 'CHANGELOG'] + end + + it "extracts a valid tag" do + extract_ref('v2.0.0/CHANGELOG').should == ['v2.0.0', 'CHANGELOG'] + end + + it "extracts a valid commit SHA" do + extract_ref('f4b14494ef6abf3d144c28e4af0c20143383e062/CHANGELOG').should == + ['f4b14494ef6abf3d144c28e4af0c20143383e062', 'CHANGELOG'] + end + + it "falls back to a primitive split for an invalid ref" do + extract_ref('stable/CHANGELOG').should == ['stable', 'CHANGELOG'] + end + end end end From 39c657930625ddc3ac8a921f01ffc83acadce68f Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Mon, 17 Sep 2012 13:36:13 -0400 Subject: [PATCH 15/65] Add BlameController, remove Refs#blame action --- app/controllers/blame_controller.rb | 45 +++++++++++++++++++ app/controllers/refs_controller.rb | 6 +-- app/views/{refs => blame}/_head.html.haml | 0 app/views/blame/show.html.haml | 2 +- app/views/tree/_tree_file.html.haml | 2 +- config/routes.rb | 10 +---- .../requests/gitlab_flavored_markdown_spec.rb | 2 +- spec/routing/project_routing_spec.rb | 17 +++---- 8 files changed, 57 insertions(+), 27 deletions(-) create mode 100644 app/controllers/blame_controller.rb rename app/views/{refs => blame}/_head.html.haml (100%) diff --git a/app/controllers/blame_controller.rb b/app/controllers/blame_controller.rb new file mode 100644 index 00000000..d159a83c --- /dev/null +++ b/app/controllers/blame_controller.rb @@ -0,0 +1,45 @@ +# Controller for viewing a file's blame +class BlameController < ApplicationController + # Thrown when given an invalid path + class InvalidPathError < StandardError; end + + include RefExtractor + + layout "project" + + before_filter :project + + # Authorize + before_filter :add_project_abilities + before_filter :authorize_read_project! + before_filter :authorize_code_access! + before_filter :require_non_empty_project + + before_filter :define_tree_vars + + def show + @blame = Grit::Blob.blame(@repo, @commit.id, @path) + end + + private + + def define_tree_vars + @ref, @path = extract_ref(params[:id]) + + @id = File.join(@ref, @path) + @repo = @project.repo + @commit = CommitDecorator.decorate(@project.commit(@ref)) + + @tree = Tree.new(@commit.tree, @project, @ref, @path) + @tree = TreeDecorator.new(@tree) + + raise InvalidPathError if @tree.invalid? + + @hex_path = Digest::SHA1.hexdigest(@path) + + @history_path = project_tree_path(@project, @id) + @logs_path = logs_file_project_ref_path(@project, @ref, @path) + rescue NoMethodError, InvalidPathError + not_found! + end +end diff --git a/app/controllers/refs_controller.rb b/app/controllers/refs_controller.rb index 6cdd0953..334dfc7f 100644 --- a/app/controllers/refs_controller.rb +++ b/app/controllers/refs_controller.rb @@ -9,7 +9,7 @@ class RefsController < ApplicationController before_filter :require_non_empty_project before_filter :ref - before_filter :define_tree_vars, only: [:tree, :blob, :blame, :logs_tree] + before_filter :define_tree_vars, only: [:blob, :logs_tree] before_filter :render_full_content layout "project" @@ -79,10 +79,6 @@ class RefsController < ApplicationController end end - def blame - @blame = Grit::Blob.blame(@repo, @commit.id, params[:path]) - end - protected def define_tree_vars diff --git a/app/views/refs/_head.html.haml b/app/views/blame/_head.html.haml similarity index 100% rename from app/views/refs/_head.html.haml rename to app/views/blame/_head.html.haml diff --git a/app/views/blame/show.html.haml b/app/views/blame/show.html.haml index 75b86315..0b2f93be 100644 --- a/app/views/blame/show.html.haml +++ b/app/views/blame/show.html.haml @@ -20,7 +20,7 @@ %span.options = link_to "raw", blob_project_ref_path(@project, @ref, path: params[:path]), class: "btn very_small", target: "_blank" = link_to "history", project_commits_path(@project, path: params[:path], ref: @ref), class: "btn very_small" - = link_to "source", project_tree_path(@project, tree_join(@ref, params[:path])), class: "btn very_small" + = link_to "source", project_tree_path(@project, @id), class: "btn very_small" .file_content.blame %table - @blame.each do |commit, lines| diff --git a/app/views/tree/_tree_file.html.haml b/app/views/tree/_tree_file.html.haml index ae8f1ccf..052172be 100644 --- a/app/views/tree/_tree_file.html.haml +++ b/app/views/tree/_tree_file.html.haml @@ -7,7 +7,7 @@ %span.options = link_to "raw", blob_project_ref_path(@project, @ref, path: @path), class: "btn very_small", target: "_blank" = link_to "history", project_commits_path(@project, path: params[:path], ref: @ref), class: "btn very_small" - = link_to "blame", blame_file_project_ref_path(@project, @ref, path: @path.gsub(/^\//, '')), class: "btn very_small" + = link_to "blame", project_blame_path(@project, @id), class: "btn very_small" - if file.text? - if gitlab_markdown?(name) .file_content.wiki diff --git a/config/routes.rb b/config/routes.rb index d1ed8749..1aa10c5d 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -136,14 +136,6 @@ Gitlab::Application.routes.draw do id: /[a-zA-Z.0-9\/_\-]+/, path: /.*/ } - - # blame - get "blame/:path" => "refs#blame", - as: :blame_file, - constraints: { - id: /[a-zA-Z.0-9\/_\-]+/, - path: /.*/ - } end end @@ -204,7 +196,7 @@ Gitlab::Application.routes.draw do end # XXX: WIP - # resources :blame, only: [:show], constraints: {id: /.+/} + resources :blame, only: [:show], constraints: {id: /.+/} # resources :blob, only: [:show], constraints: {id: /.+/} # resources :raw, only: [:show], constraints: {id: /.+/} resources :tree, only: [:show], constraints: {id: /.+/} diff --git a/spec/requests/gitlab_flavored_markdown_spec.rb b/spec/requests/gitlab_flavored_markdown_spec.rb index db3f89cb..2e5d4209 100644 --- a/spec/requests/gitlab_flavored_markdown_spec.rb +++ b/spec/requests/gitlab_flavored_markdown_spec.rb @@ -69,7 +69,7 @@ describe "Gitlab Flavored Markdown" do end it "should render title in refs#blame" do - visit blame_file_project_ref_path(project, id: @branch_name, path: @test_file) + visit blame_file_project_ref_path(project, File.join(@branch_name, @test_file)) within(".blame_commit") do page.should have_link("##{issue.id}") diff --git a/spec/routing/project_routing_spec.rb b/spec/routing/project_routing_spec.rb index f9c99a20..303df02c 100644 --- a/spec/routing/project_routing_spec.rb +++ b/spec/routing/project_routing_spec.rb @@ -195,7 +195,6 @@ end # blob_project_ref GET /:project_id/:id/blob(.:format) refs#blob # logs_tree_project_ref GET /:project_id/:id/logs_tree(.:format) refs#logs_tree # logs_file_project_ref GET /:project_id/:id/logs_tree/:path(.:format) refs#logs_tree -# blame_file_project_ref GET /:project_id/:id/blame/:path(.:format) refs#blame describe RefsController, "routing" do it "to #switch" do get("/gitlabhq/switch").should route_to('refs#switch', project_id: 'gitlabhq') @@ -209,10 +208,6 @@ describe RefsController, "routing" do it "to #blob" do get("/gitlabhq/stable/blob").should route_to('refs#blob', project_id: 'gitlabhq', id: 'stable') end - - it "to #blame" do - get("/gitlabhq/stable/blame/foo/bar/baz").should route_to('refs#blame', project_id: 'gitlabhq', id: 'stable', path: 'foo/bar/baz') - end end # diffs_project_merge_request GET /:project_id/merge_requests/:id/diffs(.:format) merge_requests#diffs @@ -399,6 +394,12 @@ describe NotesController, "routing" do end end +describe BlameController, "routing" do + it "to #show" do + get("/gitlabhq/blame/master/app/models/project.rb").should route_to('blame#show', project_id: 'gitlabhq', id: 'master/app/models/project.rb') + end +end + describe TreeController, "routing" do it "to #show" do get("/gitlabhq/tree/master/app/models/project.rb").should route_to('tree#show', project_id: 'gitlabhq', id: 'master/app/models/project.rb') @@ -432,11 +433,7 @@ end # /gitlabhq/tree/master/app # /gitlabhq/tree/test/branch/name/app describe "pending routing" do - describe "/:project_id/blame/:id" do - it "routes to a ref with a path" do - get("/gitlabhq/blame/master/app/models/project.rb").should route_to('blame#show', project_id: 'gitlabhq', id: 'master/app/models/project.rb') - end - end + before { pending } describe "/:project_id/blob/:id" do it "routes to a ref with a path" do From 576cec6c67dcc4ee00b8220ca1a45385583e25b2 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Mon, 17 Sep 2012 13:49:57 -0400 Subject: [PATCH 16/65] Add BlobController, remove Refs#blob --- app/controllers/blob_controller.rb | 62 +++++++++++++++++++ app/controllers/refs_controller.rb | 49 +++------------ app/views/blame/show.html.haml | 2 +- app/views/commits/_diffs.html.haml | 2 +- app/views/tree/_tree_file.html.haml | 6 +- config/routes.rb | 8 +-- spec/requests/security/project_access_spec.rb | 2 +- spec/routing/project_routing_spec.rb | 17 ++--- 8 files changed, 83 insertions(+), 65 deletions(-) create mode 100644 app/controllers/blob_controller.rb diff --git a/app/controllers/blob_controller.rb b/app/controllers/blob_controller.rb new file mode 100644 index 00000000..bb051281 --- /dev/null +++ b/app/controllers/blob_controller.rb @@ -0,0 +1,62 @@ +# Controller for viewing a file's blame +class BlobController < ApplicationController + # Thrown when given an invalid path + class InvalidPathError < StandardError; end + + include RefExtractor + include Gitlab::Encode + + layout "project" + + before_filter :project + + # Authorize + before_filter :add_project_abilities + before_filter :authorize_read_project! + before_filter :authorize_code_access! + before_filter :require_non_empty_project + + before_filter :define_tree_vars + + def show + if @tree.is_blob? + if @tree.text? + encoding = detect_encoding(@tree.data) + mime_type = encoding ? "text/plain; charset=#{encoding}" : "text/plain" + else + mime_type = @tree.mime_type + end + + send_data( + @tree.data, + type: mime_type, + disposition: 'inline', + filename: @tree.name + ) + else + not_found! + end + end + + private + + def define_tree_vars + @ref, @path = extract_ref(params[:id]) + + @id = File.join(@ref, @path) + @repo = @project.repo + @commit = CommitDecorator.decorate(@project.commit(@ref)) + + @tree = Tree.new(@commit.tree, @project, @ref, @path) + @tree = TreeDecorator.new(@tree) + + raise InvalidPathError if @tree.invalid? + + @hex_path = Digest::SHA1.hexdigest(@path) + + @history_path = project_tree_path(@project, @id) + @logs_path = logs_file_project_ref_path(@project, @ref, @path) + rescue NoMethodError, InvalidPathError + not_found! + end +end diff --git a/app/controllers/refs_controller.rb b/app/controllers/refs_controller.rb index 334dfc7f..f9cd4734 100644 --- a/app/controllers/refs_controller.rb +++ b/app/controllers/refs_controller.rb @@ -18,14 +18,14 @@ class RefsController < ApplicationController respond_to do |format| format.html do new_path = if params[:destination] == "tree" - project_tree_path(@project, params[:ref]) + project_tree_path(@project, @ref) else - project_commits_path(@project, ref: params[:ref]) + project_commits_path(@project, ref: @ref) end - redirect_to new_path + redirect_to new_path end - format.js do + format.js do @ref = params[:ref] define_tree_vars render "tree" @@ -33,19 +33,6 @@ class RefsController < ApplicationController end end - # - # Repository preview - # - def tree - respond_to do |format| - format.html - format.js do - # disable cache to allow back button works - no_cache_headers - end - end - end - def logs_tree contents = @tree.contents @logs = contents.map do |content| @@ -53,32 +40,12 @@ class RefsController < ApplicationController last_commit = @project.commits(@commit.id, file, 1).last last_commit = CommitDecorator.decorate(last_commit) { - file_name: content.name, + file_name: content.name, commit: last_commit } end end - def blob - if @tree.is_blob? - if @tree.text? - encoding = detect_encoding(@tree.data) - mime_type = encoding ? "text/plain; charset=#{encoding}" : "text/plain" - else - mime_type = @tree.mime_type - end - - send_data( - @tree.data, - type: mime_type, - disposition: 'inline', - filename: @tree.name - ) - else - head(404) - end - end - protected def define_tree_vars @@ -93,15 +60,15 @@ class RefsController < ApplicationController if params[:path] @history_path = project_tree_path(@project, File.join(@ref, params[:path])) - @logs_path = logs_file_project_ref_path(@project, @ref, params[:path]) + @logs_path = logs_file_project_ref_path(@project, @ref, params[:path]) else @history_path = project_tree_path(@project, @ref) - @logs_path = logs_tree_project_ref_path(@project, @ref) + @logs_path = logs_tree_project_ref_path(@project, @ref) end rescue return render_404 end - + def ref @ref = params[:id] end diff --git a/app/views/blame/show.html.haml b/app/views/blame/show.html.haml index 0b2f93be..12b4e668 100644 --- a/app/views/blame/show.html.haml +++ b/app/views/blame/show.html.haml @@ -18,7 +18,7 @@ = @tree.name %small blame %span.options - = link_to "raw", blob_project_ref_path(@project, @ref, path: params[:path]), class: "btn very_small", target: "_blank" + = link_to "raw", project_blob_path(@project, @id), class: "btn very_small", target: "_blank" = link_to "history", project_commits_path(@project, path: params[:path], ref: @ref), class: "btn very_small" = link_to "source", project_tree_path(@project, @id), class: "btn very_small" .file_content.blame diff --git a/app/views/commits/_diffs.html.haml b/app/views/commits/_diffs.html.haml index bc53911c..301cd554 100644 --- a/app/views/commits/_diffs.html.haml +++ b/app/views/commits/_diffs.html.haml @@ -24,7 +24,7 @@ %i.icon-file %span{id: "#{diff.old_path}"}= diff.old_path - else - = link_to project_tree_path(@project, @commit, diff.new_path) do + = link_to project_tree_path(@project, tree_join(@commit.id, diff.new_path)) do %i.icon-file %span{id: "#{diff.new_path}"}= diff.new_path %br/ diff --git a/app/views/tree/_tree_file.html.haml b/app/views/tree/_tree_file.html.haml index 052172be..b4c4566b 100644 --- a/app/views/tree/_tree_file.html.haml +++ b/app/views/tree/_tree_file.html.haml @@ -5,8 +5,8 @@ = name.force_encoding('utf-8') %small #{file.mode} %span.options - = link_to "raw", blob_project_ref_path(@project, @ref, path: @path), class: "btn very_small", target: "_blank" - = link_to "history", project_commits_path(@project, path: params[:path], ref: @ref), class: "btn very_small" + = link_to "raw", project_blob_path(@project, @id), class: "btn very_small", target: "_blank" + = link_to "history", project_commits_path(@project, path: @path, ref: @ref), class: "btn very_small" = link_to "blame", project_blame_path(@project, @id), class: "btn very_small" - if file.text? - if gitlab_markdown?(name) @@ -32,7 +32,7 @@ - else .file_content.blob_file %center - = link_to blob_project_ref_path(@project, @ref, path: params[:path]) do + = link_to project_blob_path(@project, @id) do %div.padded %br = image_tag "download.png", width: 64 diff --git a/config/routes.rb b/config/routes.rb index 1aa10c5d..af7b9bd4 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -122,12 +122,6 @@ Gitlab::Application.routes.draw do end member do - get "blob", - constraints: { - id: /[a-zA-Z.0-9\/_\-]+/, - path: /.*/ - } - # tree viewer logs get "logs_tree", constraints: { id: /[a-zA-Z.\/0-9_\-]+/ } get "logs_tree/:path" => "refs#logs_tree", @@ -197,7 +191,7 @@ Gitlab::Application.routes.draw do # XXX: WIP resources :blame, only: [:show], constraints: {id: /.+/} - # resources :blob, only: [:show], constraints: {id: /.+/} + resources :blob, only: [:show], constraints: {id: /.+/} # resources :raw, only: [:show], constraints: {id: /.+/} resources :tree, only: [:show], constraints: {id: /.+/} end diff --git a/spec/requests/security/project_access_spec.rb b/spec/requests/security/project_access_spec.rb index 4b6abd3e..9b07df6c 100644 --- a/spec/requests/security/project_access_spec.rb +++ b/spec/requests/security/project_access_spec.rb @@ -95,7 +95,7 @@ describe "Application access" do before do commit = @project.commit path = commit.tree.contents.select { |i| i.is_a?(Grit::Blob)}.first.name - @blob_path = blob_project_ref_path(@project, commit.id, path: path) + @blob_path = project_blob_path(@project, File.join(commit.id, path)) end it { @blob_path.should be_allowed_for @u1 } diff --git a/spec/routing/project_routing_spec.rb b/spec/routing/project_routing_spec.rb index 303df02c..01bc303d 100644 --- a/spec/routing/project_routing_spec.rb +++ b/spec/routing/project_routing_spec.rb @@ -192,7 +192,6 @@ describe ProtectedBranchesController, "routing" do end # switch_project_refs GET /:project_id/switch(.:format) refs#switch -# blob_project_ref GET /:project_id/:id/blob(.:format) refs#blob # logs_tree_project_ref GET /:project_id/:id/logs_tree(.:format) refs#logs_tree # logs_file_project_ref GET /:project_id/:id/logs_tree/:path(.:format) refs#logs_tree describe RefsController, "routing" do @@ -204,10 +203,6 @@ describe RefsController, "routing" do get("/gitlabhq/stable/logs_tree").should route_to('refs#logs_tree', project_id: 'gitlabhq', id: 'stable') get("/gitlabhq/stable/logs_tree/foo/bar/baz").should route_to('refs#logs_tree', project_id: 'gitlabhq', id: 'stable', path: 'foo/bar/baz') end - - it "to #blob" do - get("/gitlabhq/stable/blob").should route_to('refs#blob', project_id: 'gitlabhq', id: 'stable') - end end # diffs_project_merge_request GET /:project_id/merge_requests/:id/diffs(.:format) merge_requests#diffs @@ -400,6 +395,12 @@ describe BlameController, "routing" do end end +describe BlobController, "routing" do + it "to #show" do + get("/gitlabhq/blob/master/app/models/project.rb").should route_to('blob#show', project_id: 'gitlabhq', id: 'master/app/models/project.rb') + end +end + describe TreeController, "routing" do it "to #show" do get("/gitlabhq/tree/master/app/models/project.rb").should route_to('tree#show', project_id: 'gitlabhq', id: 'master/app/models/project.rb') @@ -435,12 +436,6 @@ end describe "pending routing" do before { pending } - describe "/:project_id/blob/:id" do - it "routes to a ref with a path" do - get("/gitlabhq/blob/master/app/models/project.rb").should route_to('blob#show', project_id: 'gitlabhq', id: 'master/app/models/project.rb') - end - end - describe "/:project_id/commit/:id" do it "routes to a specific commit" do get("/gitlabhq/commit/f4b1449").should route_to('commit#show', project_id: 'gitlabhq', id: 'f4b1449') From 398ba6f1bb60f176444dedc7b26188e08b920f54 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Mon, 17 Sep 2012 14:24:31 -0400 Subject: [PATCH 17/65] DRY up Blame, Blob and Tree controllers --- app/controllers/blame_controller.rb | 27 ++------------------------- app/controllers/blob_controller.rb | 24 +----------------------- app/controllers/tree_controller.rb | 29 ++++++----------------------- app/views/tree/show.html.haml | 2 +- app/views/tree/show.js.haml | 2 +- lib/ref_extractor.rb | 27 +++++++++++++++++++++++++++ 6 files changed, 38 insertions(+), 73 deletions(-) diff --git a/app/controllers/blame_controller.rb b/app/controllers/blame_controller.rb index d159a83c..dd5be6dc 100644 --- a/app/controllers/blame_controller.rb +++ b/app/controllers/blame_controller.rb @@ -1,7 +1,5 @@ # Controller for viewing a file's blame class BlameController < ApplicationController - # Thrown when given an invalid path - class InvalidPathError < StandardError; end include RefExtractor @@ -15,31 +13,10 @@ class BlameController < ApplicationController before_filter :authorize_code_access! before_filter :require_non_empty_project - before_filter :define_tree_vars + before_filter :assign_ref_vars def show + @repo = @project.repo @blame = Grit::Blob.blame(@repo, @commit.id, @path) end - - private - - def define_tree_vars - @ref, @path = extract_ref(params[:id]) - - @id = File.join(@ref, @path) - @repo = @project.repo - @commit = CommitDecorator.decorate(@project.commit(@ref)) - - @tree = Tree.new(@commit.tree, @project, @ref, @path) - @tree = TreeDecorator.new(@tree) - - raise InvalidPathError if @tree.invalid? - - @hex_path = Digest::SHA1.hexdigest(@path) - - @history_path = project_tree_path(@project, @id) - @logs_path = logs_file_project_ref_path(@project, @ref, @path) - rescue NoMethodError, InvalidPathError - not_found! - end end diff --git a/app/controllers/blob_controller.rb b/app/controllers/blob_controller.rb index bb051281..58e70bc9 100644 --- a/app/controllers/blob_controller.rb +++ b/app/controllers/blob_controller.rb @@ -16,7 +16,7 @@ class BlobController < ApplicationController before_filter :authorize_code_access! before_filter :require_non_empty_project - before_filter :define_tree_vars + before_filter :assign_ref_vars def show if @tree.is_blob? @@ -37,26 +37,4 @@ class BlobController < ApplicationController not_found! end end - - private - - def define_tree_vars - @ref, @path = extract_ref(params[:id]) - - @id = File.join(@ref, @path) - @repo = @project.repo - @commit = CommitDecorator.decorate(@project.commit(@ref)) - - @tree = Tree.new(@commit.tree, @project, @ref, @path) - @tree = TreeDecorator.new(@tree) - - raise InvalidPathError if @tree.invalid? - - @hex_path = Digest::SHA1.hexdigest(@path) - - @history_path = project_tree_path(@project, @id) - @logs_path = logs_file_project_ref_path(@project, @ref, @path) - rescue NoMethodError, InvalidPathError - not_found! - end end diff --git a/app/controllers/tree_controller.rb b/app/controllers/tree_controller.rb index 15bbb1a3..e0dd8f8b 100644 --- a/app/controllers/tree_controller.rb +++ b/app/controllers/tree_controller.rb @@ -15,35 +15,18 @@ class TreeController < ApplicationController before_filter :authorize_code_access! before_filter :require_non_empty_project - before_filter :define_tree_vars + before_filter :assign_ref_vars def show + @hex_path = Digest::SHA1.hexdigest(@path) + + @history_path = project_tree_path(@project, @id) + @logs_path = logs_file_project_ref_path(@project, @ref, @path) + respond_to do |format| format.html # Disable cache so browser history works format.js { no_cache_headers } end end - - private - - def define_tree_vars - @ref, @path = extract_ref(params[:id]) - - @id = File.join(@ref, @path) - @repo = @project.repo - @commit = CommitDecorator.decorate(@project.commit(@ref)) - - @tree = Tree.new(@commit.tree, @project, @ref, @path.gsub(/^\//, '')) - @tree = TreeDecorator.new(@tree) - - raise InvalidPathError if @tree.invalid? - - @hex_path = Digest::SHA1.hexdigest(@path) - - @history_path = project_tree_path(@project, @id) - @logs_path = logs_file_project_ref_path(@project, @ref, @path) - rescue NoMethodError, InvalidPathError - not_found! - end end diff --git a/app/views/tree/show.html.haml b/app/views/tree/show.html.haml index 181be642..d95f90e0 100644 --- a/app/views/tree/show.html.haml +++ b/app/views/tree/show.html.haml @@ -1,5 +1,5 @@ = render "head" -#tree-holder= render partial: "tree", locals: {repo: @repo, commit: @commit, tree: @tree} +#tree-holder= render partial: "tree", locals: {commit: @commit, tree: @tree} :javascript $(function() { diff --git a/app/views/tree/show.js.haml b/app/views/tree/show.js.haml index 3d0215ff..174e3e03 100644 --- a/app/views/tree/show.js.haml +++ b/app/views/tree/show.js.haml @@ -1,6 +1,6 @@ :plain // Load Files list - $("#tree-holder").html("#{escape_javascript(render(partial: "tree", locals: {repo: @repo, commit: @commit, tree: @tree}))}"); + $("#tree-holder").html("#{escape_javascript(render(partial: "tree", locals: {commit: @commit, tree: @tree}))}"); $("#tree-content-holder").show("slide", { direction: "right" }, 150); $('.project-refs-form #path').val("#{@path}"); diff --git a/lib/ref_extractor.rb b/lib/ref_extractor.rb index d7d446b1..b9c02917 100644 --- a/lib/ref_extractor.rb +++ b/lib/ref_extractor.rb @@ -67,4 +67,31 @@ module RefExtractor pair end + + # Assigns common instance variables for views working with Git tree-ish objects + # + # Assignments are: + # + # - @id - A string representing the joined ref and path + # - @ref - A string representing the ref (e.g., the branch, tag, or commit SHA) + # - @path - A string representing the filesystem path + # - @commit - A CommitDecorator representing the commit from the given ref + # - @tree - A TreeDecorator representing the tree at the given ref/path + # + # Automatically renders `not_found!` if a valid tree could not be resolved + # (e.g., when a user inserts an invalid path or ref). + def assign_ref_vars + @ref, @path = extract_ref(params[:id]) + + @id = File.join(@ref, @path) + + @commit = CommitDecorator.decorate(@project.commit(@ref)) + + @tree = Tree.new(@commit.tree, @project, @ref, @path) + @tree = TreeDecorator.new(@tree) + + raise InvalidPathError if @tree.invalid? + rescue NoMethodError, InvalidPathError + not_found! + end end From a8ea8d98a4f88a292289ddfedef4358033b68ec0 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Thu, 20 Sep 2012 13:25:28 -0400 Subject: [PATCH 18/65] Update RefExtractor to handle atom feeds --- app/controllers/tree_controller.rb | 3 --- lib/ref_extractor.rb | 8 +++++++- spec/routing/project_routing_spec.rb | 16 +++++----------- 3 files changed, 12 insertions(+), 15 deletions(-) diff --git a/app/controllers/tree_controller.rb b/app/controllers/tree_controller.rb index e0dd8f8b..43087664 100644 --- a/app/controllers/tree_controller.rb +++ b/app/controllers/tree_controller.rb @@ -1,8 +1,5 @@ # Controller for viewing a repository's file structure class TreeController < ApplicationController - # Thrown when given an invalid path - class InvalidPathError < StandardError; end - include RefExtractor layout "project" diff --git a/lib/ref_extractor.rb b/lib/ref_extractor.rb index b9c02917..1db74b95 100644 --- a/lib/ref_extractor.rb +++ b/lib/ref_extractor.rb @@ -1,7 +1,7 @@ # Module providing an extract_ref method for controllers working with Git # tree-ish + path params module RefExtractor - # Thrown when given an invalid path + # Raised when given an invalid path class InvalidPathError < StandardError; end # Given a string containing both a Git ref - such as a branch or tag - and a @@ -81,6 +81,12 @@ module RefExtractor # Automatically renders `not_found!` if a valid tree could not be resolved # (e.g., when a user inserts an invalid path or ref). def assign_ref_vars + # Handle formats embedded in the id + if params[:id].ends_with?('.atom') + params[:id].gsub!(/\.atom$/, '') + request.format = :atom + end + @ref, @path = extract_ref(params[:id]) @id = File.join(@ref, @path) diff --git a/spec/routing/project_routing_spec.rb b/spec/routing/project_routing_spec.rb index 01bc303d..04164d5d 100644 --- a/spec/routing/project_routing_spec.rb +++ b/spec/routing/project_routing_spec.rb @@ -293,11 +293,7 @@ end # patch_project_commit GET /:project_id/commits/:id/patch(.:format) commits#patch # project_commits GET /:project_id/commits(.:format) commits#index # POST /:project_id/commits(.:format) commits#create -# new_project_commit GET /:project_id/commits/new(.:format) commits#new -# edit_project_commit GET /:project_id/commits/:id/edit(.:format) commits#edit # project_commit GET /:project_id/commits/:id(.:format) commits#show -# PUT /:project_id/commits/:id(.:format) commits#update -# DELETE /:project_id/commits/:id(.:format) commits#destroy describe CommitsController, "routing" do it "to #compare" do get("/gitlabhq/commits/compare").should route_to('commits#compare', project_id: 'gitlabhq') @@ -307,6 +303,10 @@ describe CommitsController, "routing" do get("/gitlabhq/commits/1/patch").should route_to('commits#patch', project_id: 'gitlabhq', id: '1') end + it "does something with atom feeds" do + get("/gitlabhq/commits/master.atom").should route_to('commits#show', project_id: 'gitlabhq', id: 'master.atom') + end + it_behaves_like "RESTful project resources" do let(:actions) { [:index, :show] } let(:controller) { 'commits' } @@ -425,6 +425,7 @@ end # /:project_id/commits/*path # /gitlabhq/commits/master/app/contexts/base_context.rb # /gitlabhq/commits/test/branch/name/app/contexts/base_context.rb +# /gitlabhq/commits/master.atom # # /:project_id/raw/*path # /gitlabhq/raw/master/app/contexts/base_context.rb @@ -436,13 +437,6 @@ end describe "pending routing" do before { pending } - describe "/:project_id/commit/:id" do - it "routes to a specific commit" do - get("/gitlabhq/commit/f4b1449").should route_to('commit#show', project_id: 'gitlabhq', id: 'f4b1449') - get("/gitlabhq/commit/f4b14494ef6abf3d144c28e4af0c20143383e062").should route_to('commit#show', project_id: 'gitlabhq', id: 'f4b14494ef6abf3d144c28e4af0c20143383e062') - end - end - describe "/:project_id/raw/:id" do it "routes to a ref with a path" do get("/gitlabhq/raw/master/app/models/project.rb").should route_to('raw#show', project_id: 'gitlabhq', id: 'master/app/models/project.rb') From a1e68a91205186287f21fb5fd1669acebcd7e79e Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Thu, 20 Sep 2012 13:55:14 -0400 Subject: [PATCH 19/65] Rename RefExtractor to ExtractsPath Update docs a bit --- app/controllers/blame_controller.rb | 3 +- app/controllers/blob_controller.rb | 5 +- app/controllers/tree_controller.rb | 2 +- lib/{ref_extractor.rb => extracts_path.rb} | 31 ++++++++---- spec/lib/extracts_path_spec.rb | 58 ++++++++++++++++++++++ spec/lib/ref_extractor_spec.rb | 4 +- 6 files changed, 84 insertions(+), 19 deletions(-) rename lib/{ref_extractor.rb => extracts_path.rb} (77%) create mode 100644 spec/lib/extracts_path_spec.rb diff --git a/app/controllers/blame_controller.rb b/app/controllers/blame_controller.rb index dd5be6dc..dd0837ea 100644 --- a/app/controllers/blame_controller.rb +++ b/app/controllers/blame_controller.rb @@ -1,7 +1,6 @@ # Controller for viewing a file's blame class BlameController < ApplicationController - - include RefExtractor + include ExtractsPath layout "project" diff --git a/app/controllers/blob_controller.rb b/app/controllers/blob_controller.rb index 58e70bc9..33387842 100644 --- a/app/controllers/blob_controller.rb +++ b/app/controllers/blob_controller.rb @@ -1,9 +1,6 @@ # Controller for viewing a file's blame class BlobController < ApplicationController - # Thrown when given an invalid path - class InvalidPathError < StandardError; end - - include RefExtractor + include ExtractsPath include Gitlab::Encode layout "project" diff --git a/app/controllers/tree_controller.rb b/app/controllers/tree_controller.rb index 43087664..e6313783 100644 --- a/app/controllers/tree_controller.rb +++ b/app/controllers/tree_controller.rb @@ -1,6 +1,6 @@ # Controller for viewing a repository's file structure class TreeController < ApplicationController - include RefExtractor + include ExtractsPath layout "project" diff --git a/lib/ref_extractor.rb b/lib/extracts_path.rb similarity index 77% rename from lib/ref_extractor.rb rename to lib/extracts_path.rb index 1db74b95..6648ffdc 100644 --- a/lib/ref_extractor.rb +++ b/lib/extracts_path.rb @@ -1,14 +1,22 @@ -# Module providing an extract_ref method for controllers working with Git -# tree-ish + path params -module RefExtractor - # Raised when given an invalid path +# Module providing methods for dealing with separating a tree-ish string and a +# file path string when combined in a request parameter +module ExtractsPath + extend ActiveSupport::Concern + + # Raised when given an invalid file path class InvalidPathError < StandardError; end - # Given a string containing both a Git ref - such as a branch or tag - and a - # filesystem path joined by forward slashes, attempts to separate the two. + included do + if respond_to?(:before_filter) + before_filter :assign_ref_vars + end + end + + # Given a string containing both a Git tree-ish, such as a branch or tag, and + # a filesystem path joined by forward slashes, attempts to separate the two. # - # Expects a @project instance variable to contain the active project. Used to - # check the input against a list of valid repository refs. + # Expects a @project instance variable to contain the active project. This is + # used to check the input against a list of valid repository refs. # # Examples # @@ -78,8 +86,11 @@ module RefExtractor # - @commit - A CommitDecorator representing the commit from the given ref # - @tree - A TreeDecorator representing the tree at the given ref/path # - # Automatically renders `not_found!` if a valid tree could not be resolved - # (e.g., when a user inserts an invalid path or ref). + # If the :id parameter appears to be requesting a specific response format, + # that will be handled as well. + # + # Automatically renders `not_found!` if a valid tree path could not be + # resolved (e.g., when a user inserts an invalid path or ref). def assign_ref_vars # Handle formats embedded in the id if params[:id].ends_with?('.atom') diff --git a/spec/lib/extracts_path_spec.rb b/spec/lib/extracts_path_spec.rb new file mode 100644 index 00000000..8876373d --- /dev/null +++ b/spec/lib/extracts_path_spec.rb @@ -0,0 +1,58 @@ +require 'spec_helper' + +describe ExtractsPath do + include ExtractsPath + + let(:project) { double('project') } + + before do + @project = project + project.stub(:branches).and_return(['master', 'foo/bar/baz']) + project.stub(:tags).and_return(['v1.0.0', 'v2.0.0']) + end + + describe '#extract_ref' do + it "returns an empty pair when no @project is set" do + @project = nil + extract_ref('master/CHANGELOG').should == ['', ''] + end + + context "without a path" do + it "extracts a valid branch" do + extract_ref('master').should == ['master', ''] + end + + it "extracts a valid tag" do + extract_ref('v2.0.0').should == ['v2.0.0', ''] + end + + it "extracts a valid commit ref without a path" do + extract_ref('f4b14494ef6abf3d144c28e4af0c20143383e062').should == + ['f4b14494ef6abf3d144c28e4af0c20143383e062', ''] + end + + it "falls back to a primitive split for an invalid ref" do + extract_ref('stable').should == ['stable', ''] + end + end + + context "with a path" do + it "extracts a valid branch" do + extract_ref('foo/bar/baz/CHANGELOG').should == ['foo/bar/baz', 'CHANGELOG'] + end + + it "extracts a valid tag" do + extract_ref('v2.0.0/CHANGELOG').should == ['v2.0.0', 'CHANGELOG'] + end + + it "extracts a valid commit SHA" do + extract_ref('f4b14494ef6abf3d144c28e4af0c20143383e062/CHANGELOG').should == + ['f4b14494ef6abf3d144c28e4af0c20143383e062', 'CHANGELOG'] + end + + it "falls back to a primitive split for an invalid ref" do + extract_ref('stable/CHANGELOG').should == ['stable', 'CHANGELOG'] + end + end + end +end diff --git a/spec/lib/ref_extractor_spec.rb b/spec/lib/ref_extractor_spec.rb index 1e3d178f..8876373d 100644 --- a/spec/lib/ref_extractor_spec.rb +++ b/spec/lib/ref_extractor_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' -describe RefExtractor do - include RefExtractor +describe ExtractsPath do + include ExtractsPath let(:project) { double('project') } From 5a5d214de499b678802ac12c5c6cbe583a85ae36 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Thu, 20 Sep 2012 14:43:38 -0400 Subject: [PATCH 20/65] Remove unused render_full_content filter --- app/controllers/application_controller.rb | 4 ---- app/controllers/commits_controller.rb | 1 - app/controllers/protected_branches_controller.rb | 1 - app/controllers/refs_controller.rb | 1 - app/controllers/repositories_controller.rb | 1 - app/controllers/snippets_controller.rb | 1 - 6 files changed, 9 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 5ac5c639..a3eb3e3e 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -126,10 +126,6 @@ class ApplicationController < ActionController::Base response.headers["Expires"] = "Fri, 01 Jan 1990 00:00:00 GMT" end - def render_full_content - @full_content = true - end - def dev_tools Rack::MiniProfiler.authorize_request end diff --git a/app/controllers/commits_controller.rb b/app/controllers/commits_controller.rb index 83404bdc..712b842c 100644 --- a/app/controllers/commits_controller.rb +++ b/app/controllers/commits_controller.rb @@ -10,7 +10,6 @@ class CommitsController < ApplicationController before_filter :authorize_code_access! before_filter :require_non_empty_project before_filter :load_refs, only: :index # load @branch, @tag & @ref - before_filter :render_full_content def index @repo = project.repo diff --git a/app/controllers/protected_branches_controller.rb b/app/controllers/protected_branches_controller.rb index 78c9c9ef..2556f095 100644 --- a/app/controllers/protected_branches_controller.rb +++ b/app/controllers/protected_branches_controller.rb @@ -7,7 +7,6 @@ class ProtectedBranchesController < ApplicationController before_filter :require_non_empty_project before_filter :authorize_admin_project!, only: [:destroy, :create] - before_filter :render_full_content layout "project" diff --git a/app/controllers/refs_controller.rb b/app/controllers/refs_controller.rb index f9cd4734..5e36be30 100644 --- a/app/controllers/refs_controller.rb +++ b/app/controllers/refs_controller.rb @@ -10,7 +10,6 @@ class RefsController < ApplicationController before_filter :ref before_filter :define_tree_vars, only: [:blob, :logs_tree] - before_filter :render_full_content layout "project" diff --git a/app/controllers/repositories_controller.rb b/app/controllers/repositories_controller.rb index cd20677e..583edf8e 100644 --- a/app/controllers/repositories_controller.rb +++ b/app/controllers/repositories_controller.rb @@ -6,7 +6,6 @@ class RepositoriesController < ApplicationController before_filter :authorize_read_project! before_filter :authorize_code_access! before_filter :require_non_empty_project - before_filter :render_full_content layout "project" diff --git a/app/controllers/snippets_controller.rb b/app/controllers/snippets_controller.rb index b00c9283..a38fd52f 100644 --- a/app/controllers/snippets_controller.rb +++ b/app/controllers/snippets_controller.rb @@ -55,7 +55,6 @@ class SnippetsController < ApplicationController def show @note = @project.notes.new(noteable: @snippet) - render_full_content end def destroy From 169f16fb32091701bcaa962872ea35b5772a1539 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Thu, 20 Sep 2012 15:20:48 -0400 Subject: [PATCH 21/65] Remove Commits#compare, add CompareController --- app/controllers/commits_controller.rb | 12 ---------- app/controllers/compare_controller.rb | 22 ++++++++++++++++++ app/views/compare/_head.html.haml | 23 +++++++++++++++++++ .../show.html.haml} | 0 config/routes.rb | 5 +--- spec/routing/project_routing_spec.rb | 12 ++++++---- 6 files changed, 53 insertions(+), 21 deletions(-) create mode 100644 app/controllers/compare_controller.rb create mode 100644 app/views/compare/_head.html.haml rename app/views/{commits/compare.html.haml => compare/show.html.haml} (100%) diff --git a/app/controllers/commits_controller.rb b/app/controllers/commits_controller.rb index 712b842c..b508d287 100644 --- a/app/controllers/commits_controller.rb +++ b/app/controllers/commits_controller.rb @@ -45,18 +45,6 @@ class CommitsController < ApplicationController # end # end - def compare - result = Commit.compare(project, params[:from], params[:to]) - - @commits = result[:commits] - @commit = result[:commit] - @diffs = result[:diffs] - @refs_are_same = result[:same] - @line_notes = [] - - @commits = CommitDecorator.decorate(@commits) - end - def patch @commit = project.commit(params[:id]) diff --git a/app/controllers/compare_controller.rb b/app/controllers/compare_controller.rb new file mode 100644 index 00000000..1124fd0c --- /dev/null +++ b/app/controllers/compare_controller.rb @@ -0,0 +1,22 @@ +class CompareController < ApplicationController + before_filter :project + layout "project" + + # Authorize + before_filter :add_project_abilities + before_filter :authorize_read_project! + before_filter :authorize_code_access! + before_filter :require_non_empty_project + + def show + result = Commit.compare(project, params[:from], params[:to]) + + @commits = result[:commits] + @commit = result[:commit] + @diffs = result[:diffs] + @refs_are_same = result[:same] + @line_notes = [] + + @commits = CommitDecorator.decorate(@commits) + end +end diff --git a/app/views/compare/_head.html.haml b/app/views/compare/_head.html.haml new file mode 100644 index 00000000..a8111a72 --- /dev/null +++ b/app/views/compare/_head.html.haml @@ -0,0 +1,23 @@ +%ul.nav.nav-tabs + %li= render partial: 'shared/ref_switcher', locals: {destination: 'commits'} + %li{class: "#{'active' if current_page?(project_commits_path(@project)) }"} + = link_to project_commits_path(@project) do + Commits + %li{class: "#{'active' if current_page?(compare_project_commits_path(@project)) }"} + = link_to compare_project_commits_path(@project) do + Compare + %li{class: "#{branches_tab_class}"} + = link_to project_repository_path(@project) do + Branches + %span.badge= @project.repo.branch_count + + %li{class: "#{'active' if current_page?(tags_project_repository_path(@project)) }"} + = link_to tags_project_repository_path(@project) do + Tags + %span.badge= @project.repo.tag_count + + - if current_page?(project_commits_path(@project)) && current_user.private_token + %li.right + %span.rss-icon + = link_to project_commits_path(@project, :atom, { private_token: current_user.private_token, ref: @ref }), title: "Feed" do + = image_tag "rss_ui.png", title: "feed" diff --git a/app/views/commits/compare.html.haml b/app/views/compare/show.html.haml similarity index 100% rename from app/views/commits/compare.html.haml rename to app/views/compare/show.html.haml diff --git a/config/routes.rb b/config/routes.rb index af7b9bd4..708dd7d5 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -162,10 +162,6 @@ Gitlab::Application.routes.draw do resources :commit, only: [:show], constraints: {id: /[[:alnum:]]{6,40}/} resources :commits, only: [:index, :show] do - collection do - get :compare - end - member do get :patch end @@ -194,6 +190,7 @@ Gitlab::Application.routes.draw do resources :blob, only: [:show], constraints: {id: /.+/} # resources :raw, only: [:show], constraints: {id: /.+/} resources :tree, only: [:show], constraints: {id: /.+/} + match "/compare/:from...:to" => "compare#show", as: "compare", constraints: {from: /.+/, to: /.+/} end root to: "dashboard#index" diff --git a/spec/routing/project_routing_spec.rb b/spec/routing/project_routing_spec.rb index 04164d5d..2939f2fc 100644 --- a/spec/routing/project_routing_spec.rb +++ b/spec/routing/project_routing_spec.rb @@ -289,16 +289,11 @@ describe CommitController, "routing" do end end -# compare_project_commits GET /:project_id/commits/compare(.:format) commits#compare # patch_project_commit GET /:project_id/commits/:id/patch(.:format) commits#patch # project_commits GET /:project_id/commits(.:format) commits#index # POST /:project_id/commits(.:format) commits#create # project_commit GET /:project_id/commits/:id(.:format) commits#show describe CommitsController, "routing" do - it "to #compare" do - get("/gitlabhq/commits/compare").should route_to('commits#compare', project_id: 'gitlabhq') - end - it "to #patch" do get("/gitlabhq/commits/1/patch").should route_to('commits#patch', project_id: 'gitlabhq', id: '1') end @@ -407,6 +402,13 @@ describe TreeController, "routing" do end end +describe CompareController, "routing" do + it "to #show" do + get("/gitlabhq/compare/master...stable").should route_to('compare#show', project_id: 'gitlabhq', from: 'master', to: 'stable') + get("/gitlabhq/compare/issue/1234...stable").should route_to('compare#show', project_id: 'gitlabhq', from: 'issue/1234', to: 'stable') + end +end + # TODO: Pending # # /:project_id/blame/*path From 3574826920eb958c5e59e86bb79b0773c187abfb Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Thu, 20 Sep 2012 15:32:00 -0400 Subject: [PATCH 22/65] Use Commits#show instead of Commits#index Takes tree-ish + path as ID --- app/controllers/commits_controller.rb | 36 +++---------------- .../{index.atom.builder => show.atom.builder} | 0 .../{index.html.haml => show.html.haml} | 0 .../commits/{index.js.haml => show.js.haml} | 0 app/views/layouts/_project_menu.html.haml | 2 +- config/routes.rb | 26 +++++++------- 6 files changed, 19 insertions(+), 45 deletions(-) rename app/views/commits/{index.atom.builder => show.atom.builder} (100%) rename app/views/commits/{index.html.haml => show.html.haml} (100%) rename app/views/commits/{index.js.haml => show.js.haml} (100%) diff --git a/app/controllers/commits_controller.rb b/app/controllers/commits_controller.rb index b508d287..ba6c6c24 100644 --- a/app/controllers/commits_controller.rb +++ b/app/controllers/commits_controller.rb @@ -4,18 +4,19 @@ class CommitsController < ApplicationController before_filter :project layout "project" + include ExtractsPath + # Authorize before_filter :add_project_abilities before_filter :authorize_read_project! before_filter :authorize_code_access! before_filter :require_non_empty_project - before_filter :load_refs, only: :index # load @branch, @tag & @ref - def index - @repo = project.repo + def show + @repo = @project.repo @limit, @offset = (params[:limit] || 40), (params[:offset] || 0) - @commits = @project.commits(@ref, params[:path], @limit, @offset) + @commits = @project.commits(@ref, @path, @limit, @offset) @commits = CommitDecorator.decorate(@commits) respond_to do |format| @@ -25,26 +26,6 @@ class CommitsController < ApplicationController end end - # def show - # result = CommitLoad.new(project, current_user, params).execute - - # @commit = result[:commit] - - # if @commit - # @suppress_diff = result[:suppress_diff] - # @note = result[:note] - # @line_notes = result[:line_notes] - # @notes_count = result[:notes_count] - # @comments_allowed = true - # else - # return git_not_found! - # end - - # if result[:status] == :huge_commit - # render "huge_commit" and return - # end - # end - def patch @commit = project.commit(params[:id]) @@ -55,11 +36,4 @@ class CommitsController < ApplicationController filename: "#{@commit.id}.patch" ) end - - protected - - def load_refs - @ref ||= params[:ref].presence || params[:branch].presence || params[:tag].presence - @ref ||= @ref || @project.try(:default_branch) || 'master' - end end diff --git a/app/views/commits/index.atom.builder b/app/views/commits/show.atom.builder similarity index 100% rename from app/views/commits/index.atom.builder rename to app/views/commits/show.atom.builder diff --git a/app/views/commits/index.html.haml b/app/views/commits/show.html.haml similarity index 100% rename from app/views/commits/index.html.haml rename to app/views/commits/show.html.haml diff --git a/app/views/commits/index.js.haml b/app/views/commits/show.js.haml similarity index 100% rename from app/views/commits/index.js.haml rename to app/views/commits/show.js.haml diff --git a/app/views/layouts/_project_menu.html.haml b/app/views/layouts/_project_menu.html.haml index 11e632b9..a46d59a3 100644 --- a/app/views/layouts/_project_menu.html.haml +++ b/app/views/layouts/_project_menu.html.haml @@ -8,7 +8,7 @@ = link_to project_tree_path(@project, @project.root_ref) do Files %li{class: commit_tab_class} - = link_to "Commits", project_commits_path(@project) + = link_to "Commits", project_history_path(@project, @project.root_ref) %li{class: tab_class(:network)} = link_to "Network", graph_project_path(@project) diff --git a/config/routes.rb b/config/routes.rb index 708dd7d5..01889bb2 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -159,13 +159,20 @@ Gitlab::Application.routes.draw do end end - resources :commit, only: [:show], constraints: {id: /[[:alnum:]]{6,40}/} + # XXX: WIP + resources :commit, only: [:show], constraints: {id: /[[:alnum:]]{6,40}/} + resources :commits, only: [:show], constraints: {id: /.+/}, as: 'history' + resources :blame, only: [:show], constraints: {id: /.+/} + resources :blob, only: [:show], constraints: {id: /.+/} + # resources :raw, only: [:show], constraints: {id: /.+/} + resources :tree, only: [:show], constraints: {id: /.+/} + match "/compare/:from...:to" => "compare#show", as: "compare", constraints: {from: /.+/, to: /.+/} - resources :commits, only: [:index, :show] do - member do - get :patch - end - end + # resources :commits, only: [:show], as: 'history' do + # member do + # get :patch + # end + # end resources :team, controller: 'team_members', only: [:index] resources :team_members @@ -184,13 +191,6 @@ Gitlab::Application.routes.draw do post :preview end end - - # XXX: WIP - resources :blame, only: [:show], constraints: {id: /.+/} - resources :blob, only: [:show], constraints: {id: /.+/} - # resources :raw, only: [:show], constraints: {id: /.+/} - resources :tree, only: [:show], constraints: {id: /.+/} - match "/compare/:from...:to" => "compare#show", as: "compare", constraints: {from: /.+/, to: /.+/} end root to: "dashboard#index" From 835be5b1bf25fc0509ef1b1f7f7f2e3a344ff231 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Tue, 25 Sep 2012 18:20:52 -0400 Subject: [PATCH 23/65] ExtractPaths - Only call assign_ref_vars on show action --- lib/extracts_path.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/extracts_path.rb b/lib/extracts_path.rb index 6648ffdc..f36dae52 100644 --- a/lib/extracts_path.rb +++ b/lib/extracts_path.rb @@ -8,7 +8,7 @@ module ExtractsPath included do if respond_to?(:before_filter) - before_filter :assign_ref_vars + before_filter :assign_ref_vars, only: [:show] end end From f1c6bd8df3225d33af4d3d032cc98be86317b348 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Tue, 25 Sep 2012 18:21:04 -0400 Subject: [PATCH 24/65] Factories - Format project path and code --- spec/factories.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/factories.rb b/spec/factories.rb index 92790a3f..e11e6d07 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -42,8 +42,8 @@ FactoryGirl.define do factory :project do sequence(:name) { |n| "project#{n}" } - path { name } - code { name } + path { name.downcase.gsub(/\s/, '_') } + code { name.downcase.gsub(/\s/, '_') } owner end From 9d394250a8055f31d81e5538cb77f19ea6d0d79b Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Tue, 25 Sep 2012 18:45:13 -0400 Subject: [PATCH 25/65] Add an inflector to mark "commits" as uncountable --- config/initializers/inflections.rb | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/config/initializers/inflections.rb b/config/initializers/inflections.rb index 9e8b0131..5d46ece1 100644 --- a/config/initializers/inflections.rb +++ b/config/initializers/inflections.rb @@ -8,3 +8,24 @@ # inflect.irregular 'person', 'people' # inflect.uncountable %w( fish sheep ) # end + +# Mark "commits" as uncountable. +# +# Without this change, the routes +# +# resources :commit, only: [:show], constraints: {id: /[[:alnum:]]{6,40}/} +# resources :commits, only: [:show], constraints: {id: /.+/} +# +# would generate identical route helper methods (`project_commit_path`), resulting +# in one of them not getting a helper method at all. +# +# After this change, the helper methods are: +# +# project_commit_path(@project, @project.commit) +# # => "/gitlabhq/commit/bcf03b5de6c33f3869ef70d68cf06e679d1d7f9a +# +# project_commits_path(@project, 'stable/README.md') +# # => "/gitlabhq/commits/stable/README.md" +ActiveSupport::Inflector.inflections do |inflect| + inflect.uncountable %w(commits) +end From c058e3903e01b950d19fe8ce86702f24083c8395 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Tue, 25 Sep 2012 18:45:30 -0400 Subject: [PATCH 26/65] Finalize new routes --- config/routes.rb | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/config/routes.rb b/config/routes.rb index 01889bb2..bc29314e 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -161,31 +161,25 @@ Gitlab::Application.routes.draw do # XXX: WIP resources :commit, only: [:show], constraints: {id: /[[:alnum:]]{6,40}/} - resources :commits, only: [:show], constraints: {id: /.+/}, as: 'history' + resources :commits, only: [:show], constraints: {id: /.+/} + resources :compare, only: [:index] resources :blame, only: [:show], constraints: {id: /.+/} resources :blob, only: [:show], constraints: {id: /.+/} - # resources :raw, only: [:show], constraints: {id: /.+/} - resources :tree, only: [:show], constraints: {id: /.+/} + resources :tree, only: [:show], constraints: {id: /.+/} match "/compare/:from...:to" => "compare#show", as: "compare", constraints: {from: /.+/, to: /.+/} - # resources :commits, only: [:show], as: 'history' do - # member do - # get :patch - # end - # end - resources :team, controller: 'team_members', only: [:index] resources :team_members resources :milestones resources :labels, only: [:index] resources :issues do - collection do post :sort post :bulk_update get :search end end + resources :notes, only: [:index, :create, :destroy] do collection do post :preview From 99d391332fa9a6c11e93bd19295425167661b972 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Tue, 25 Sep 2012 18:46:19 -0400 Subject: [PATCH 27/65] Add a "patch" MIME type, and render it like a normal view in Commit#show --- app/controllers/commit_controller.rb | 25 ++++++++++++++----------- app/controllers/commits_controller.rb | 11 ----------- app/views/commit/show.patch.erb | 1 + config/initializers/mime_types.rb | 2 ++ 4 files changed, 17 insertions(+), 22 deletions(-) create mode 100644 app/views/commit/show.patch.erb diff --git a/app/controllers/commit_controller.rb b/app/controllers/commit_controller.rb index 73a55614..de0d5b2e 100644 --- a/app/controllers/commit_controller.rb +++ b/app/controllers/commit_controller.rb @@ -15,19 +15,22 @@ class CommitController < ApplicationController result = CommitLoad.new(project, current_user, params).execute @commit = result[:commit] + git_not_found! unless @commit - if @commit - @suppress_diff = result[:suppress_diff] - @note = result[:note] - @line_notes = result[:line_notes] - @notes_count = result[:notes_count] - @comments_allowed = true - else - return git_not_found! - end + @suppress_diff = result[:suppress_diff] + @note = result[:note] + @line_notes = result[:line_notes] + @notes_count = result[:notes_count] + @comments_allowed = true - if result[:status] == :huge_commit - render "huge_commit" and return + respond_to do |format| + format.html do + if result[:status] == :huge_commit + render "huge_commit" and return + end + end + + format.patch end end end diff --git a/app/controllers/commits_controller.rb b/app/controllers/commits_controller.rb index ba6c6c24..bd67ee88 100644 --- a/app/controllers/commits_controller.rb +++ b/app/controllers/commits_controller.rb @@ -25,15 +25,4 @@ class CommitsController < ApplicationController format.atom { render layout: false } end end - - def patch - @commit = project.commit(params[:id]) - - send_data( - @commit.to_patch, - type: "text/plain", - disposition: 'attachment', - filename: "#{@commit.id}.patch" - ) - end end diff --git a/app/views/commit/show.patch.erb b/app/views/commit/show.patch.erb new file mode 100644 index 00000000..ce1c3d02 --- /dev/null +++ b/app/views/commit/show.patch.erb @@ -0,0 +1 @@ +<%= @commit.to_patch %> diff --git a/config/initializers/mime_types.rb b/config/initializers/mime_types.rb index 72aca7e4..3549b836 100644 --- a/config/initializers/mime_types.rb +++ b/config/initializers/mime_types.rb @@ -3,3 +3,5 @@ # Add new mime types for use in respond_to blocks: # Mime::Type.register "text/richtext", :rtf # Mime::Type.register_alias "text/html", :iphone + +Mime::Type.register_alias 'text/plain', :patch From 1048917232992671b0a01f4c87ed00edfff633e8 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Tue, 25 Sep 2012 18:50:02 -0400 Subject: [PATCH 28/65] Update usages of project_commit[s] route helpers --- app/controllers/refs_controller.rb | 2 +- app/decorators/event_decorator.rb | 2 +- app/decorators/tree_decorator.rb | 2 +- app/views/commits/_commit_box.html.haml | 2 +- app/views/commits/_diffs.html.haml | 2 +- app/views/layouts/_head.html.haml | 4 ++-- app/views/layouts/_project_menu.html.haml | 2 +- features/steps/shared/paths.rb | 2 +- 8 files changed, 9 insertions(+), 9 deletions(-) diff --git a/app/controllers/refs_controller.rb b/app/controllers/refs_controller.rb index 5e36be30..b260be0a 100644 --- a/app/controllers/refs_controller.rb +++ b/app/controllers/refs_controller.rb @@ -19,7 +19,7 @@ class RefsController < ApplicationController new_path = if params[:destination] == "tree" project_tree_path(@project, @ref) else - project_commits_path(@project, ref: @ref) + project_commits_path(@project, @ref) end redirect_to new_path diff --git a/app/decorators/event_decorator.rb b/app/decorators/event_decorator.rb index ce0aaa03..312ac651 100644 --- a/app/decorators/event_decorator.rb +++ b/app/decorators/event_decorator.rb @@ -21,7 +21,7 @@ class EventDecorator < ApplicationDecorator elsif self.merge_request? h.project_merge_request_url(self.project, self.merge_request) elsif self.push? - h.project_commits_url(self.project, ref: self.ref_name) + h.project_commits_url(self.project, self.ref_name) end end end diff --git a/app/decorators/tree_decorator.rb b/app/decorators/tree_decorator.rb index 05f029d0..97981785 100644 --- a/app/decorators/tree_decorator.rb +++ b/app/decorators/tree_decorator.rb @@ -30,7 +30,7 @@ class TreeDecorator < ApplicationDecorator end def history_path - h.project_commits_path(project, path: path, ref: ref) + h.project_commits_path(project, h.tree_join(ref, path)) end def mb_size diff --git a/app/views/commits/_commit_box.html.haml b/app/views/commits/_commit_box.html.haml index 3daf63f4..b8151b56 100644 --- a/app/views/commits/_commit_box.html.haml +++ b/app/views/commits/_commit_box.html.haml @@ -5,7 +5,7 @@ %span.btn.disabled.grouped %i.icon-comment = @notes_count - = link_to patch_project_commit_path(@project, @commit.id), class: "btn small grouped" do + = link_to project_commit_path(@project, @commit, format: :patch), class: "btn small grouped" do %i.icon-download-alt Get Patch = link_to project_tree_path(@project, @commit), class: "browse-button primary grouped" do diff --git a/app/views/commits/_diffs.html.haml b/app/views/commits/_diffs.html.haml index 301cd554..026fe27e 100644 --- a/app/views/commits/_diffs.html.haml +++ b/app/views/commits/_diffs.html.haml @@ -5,7 +5,7 @@ %p To prevent performance issue we rejected diff information. %p But if you still want to see diff - = link_to "click this link", project_commit_path(@project, @commit.id, force_show_diff: true), class: "dark" + = link_to "click this link", project_commit_path(@project, @commit, force_show_diff: true), class: "dark" %p.cgray Showing #{pluralize(diffs.count, "changed file")} diff --git a/app/views/layouts/_head.html.haml b/app/views/layouts/_head.html.haml index 06da82d3..1674be27 100644 --- a/app/views/layouts/_head.html.haml +++ b/app/views/layouts/_head.html.haml @@ -10,8 +10,8 @@ - if controller_name == 'projects' && action_name == 'index' = auto_discovery_link_tag :atom, projects_url(:atom, private_token: current_user.private_token), title: "Dashboard feed" - if @project && !@project.new_record? - - if current_page?(project_tree_path(@project, @project.root_ref)) || current_page?(project_commits_path(@project)) - = auto_discovery_link_tag(:atom, project_commits_url(@project, :atom, ref: @ref, private_token: current_user.private_token), title: "Recent commits to #{@project.name}:#{@ref}") + - if current_page?(project_tree_path(@project, @ref)) || current_page?(project_commits_path(@project, @ref)) + = auto_discovery_link_tag(:atom, project_commits_url(@project, @ref, format: :atom, private_token: current_user.private_token), title: "Recent commits to #{@project.name}:#{@ref}") - if request.path == project_issues_path(@project) = auto_discovery_link_tag(:atom, project_issues_url(@project, :atom, private_token: current_user.private_token), title: "#{@project.name} issues") = csrf_meta_tags diff --git a/app/views/layouts/_project_menu.html.haml b/app/views/layouts/_project_menu.html.haml index a46d59a3..13a4d037 100644 --- a/app/views/layouts/_project_menu.html.haml +++ b/app/views/layouts/_project_menu.html.haml @@ -8,7 +8,7 @@ = link_to project_tree_path(@project, @project.root_ref) do Files %li{class: commit_tab_class} - = link_to "Commits", project_history_path(@project, @project.root_ref) + = link_to "Commits", project_commits_path(@project, @project.root_ref) %li{class: tab_class(:network)} = link_to "Network", graph_project_path(@project) diff --git a/features/steps/shared/paths.rb b/features/steps/shared/paths.rb index 1839bc81..e1e1a718 100644 --- a/features/steps/shared/paths.rb +++ b/features/steps/shared/paths.rb @@ -47,7 +47,7 @@ module SharedPaths end Given 'I visit project commits page' do - visit project_commits_path(@project) + visit project_commits_path(@project, @project.root_ref) end Given 'I visit project source page' do From 33126227af662a454538d5a64762d19165f3c65d Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Tue, 25 Sep 2012 19:16:46 -0400 Subject: [PATCH 29/65] Remove check_token_auth filter Because of the way ExtractPaths works, `params[:format]` wouldn't necessarily be available at the time this filter was running, and so it would erroneously redirect to `new_user_session_path` --- app/controllers/application_controller.rb | 8 -------- 1 file changed, 8 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index a3eb3e3e..f5d978a7 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -2,7 +2,6 @@ class ApplicationController < ActionController::Base before_filter :authenticate_user! before_filter :reject_blocked! before_filter :set_current_user_for_mailer - before_filter :check_token_auth before_filter :set_current_user_for_observers before_filter :dev_tools if Rails.env == 'development' @@ -26,13 +25,6 @@ class ApplicationController < ActionController::Base protected - def check_token_auth - # Redirect to login page if not atom feed - if params[:private_token].present? && params[:format] != 'atom' - redirect_to new_user_session_path - end - end - def reject_blocked! if current_user && current_user.blocked sign_out current_user From 95f0a4114139d49b3979d6f0e53baa53193a624a Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Tue, 25 Sep 2012 19:17:54 -0400 Subject: [PATCH 30/65] Fix Refs#switch --- app/controllers/refs_controller.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app/controllers/refs_controller.rb b/app/controllers/refs_controller.rb index b260be0a..ff143f3f 100644 --- a/app/controllers/refs_controller.rb +++ b/app/controllers/refs_controller.rb @@ -13,9 +13,9 @@ class RefsController < ApplicationController layout "project" - def switch - respond_to do |format| - format.html do + def switch + respond_to do |format| + format.html do new_path = if params[:destination] == "tree" project_tree_path(@project, @ref) else @@ -69,6 +69,6 @@ class RefsController < ApplicationController end def ref - @ref = params[:id] + @ref = params[:ref] end end From 3ad931ca9211d2ca0f345f97db00193ee5533dfd Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Tue, 25 Sep 2012 19:22:44 -0400 Subject: [PATCH 31/65] Add current_controller? helper method Simplifies some of the "active tab" checks we're doing --- app/helpers/application_helper.rb | 11 +++++++++++ spec/helpers/application_helper_spec.rb | 14 ++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index d90fb32f..505f6067 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -1,6 +1,17 @@ require 'digest/md5' module ApplicationHelper + # Check if a particular controller is the current one + # + # Examples + # + # # On TreeController + # current_controller?(:tree) # => true + # current_controller?(:commits) # => false + def current_controller?(name) + controller.controller_name == name.to_s.downcase + end + def gravatar_icon(user_email = '', size = 40) if Gitlab.config.disable_gravatar? || user_email.blank? 'no_avatar.png' diff --git a/spec/helpers/application_helper_spec.rb b/spec/helpers/application_helper_spec.rb index 9a2df314..10250c93 100644 --- a/spec/helpers/application_helper_spec.rb +++ b/spec/helpers/application_helper_spec.rb @@ -1,6 +1,20 @@ require 'spec_helper' describe ApplicationHelper do + describe 'current_controller?' do + before do + controller.stub!(:controller_name).and_return('foo') + end + + it "returns true when controller matches argument" do + current_controller?(:foo).should be_true + end + + it "returns false when controller does not match argument" do + current_controller?(:bar).should_not be_true + end + end + describe "gravatar_icon" do let(:user_email) { 'user@email.com' } From 545dc7e47d627a5ba4b4219880801f26fbb77f4d Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Tue, 25 Sep 2012 19:25:14 -0400 Subject: [PATCH 32/65] Fix atom feed links --- app/views/commits/_head.html.haml | 2 +- app/views/commits/show.atom.builder | 4 ++-- app/views/compare/_head.html.haml | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/views/commits/_head.html.haml b/app/views/commits/_head.html.haml index a8111a72..cdee391f 100644 --- a/app/views/commits/_head.html.haml +++ b/app/views/commits/_head.html.haml @@ -19,5 +19,5 @@ - if current_page?(project_commits_path(@project)) && current_user.private_token %li.right %span.rss-icon - = link_to project_commits_path(@project, :atom, { private_token: current_user.private_token, ref: @ref }), title: "Feed" do + = link_to project_commits_path(@project, @ref, {format: :atom, private_token: current_user.private_token}), title: "Feed" do = image_tag "rss_ui.png", title: "feed" diff --git a/app/views/commits/show.atom.builder b/app/views/commits/show.atom.builder index cca70456..46f9838e 100644 --- a/app/views/commits/show.atom.builder +++ b/app/views/commits/show.atom.builder @@ -1,8 +1,8 @@ xml.instruct! xml.feed "xmlns" => "http://www.w3.org/2005/Atom", "xmlns:media" => "http://search.yahoo.com/mrss/" do xml.title "Recent commits to #{@project.name}:#{@ref}" - xml.link :href => project_commits_url(@project, :atom, :ref => @ref), :rel => "self", :type => "application/atom+xml" - xml.link :href => project_commits_url(@project), :rel => "alternate", :type => "text/html" + xml.link :href => project_commits_url(@project, @ref, format: :atom), :rel => "self", :type => "application/atom+xml" + xml.link :href => project_commits_url(@project, @ref), :rel => "alternate", :type => "text/html" xml.id project_commits_url(@project) xml.updated @commits.first.committed_date.strftime("%Y-%m-%dT%H:%M:%SZ") if @commits.any? diff --git a/app/views/compare/_head.html.haml b/app/views/compare/_head.html.haml index a8111a72..cdee391f 100644 --- a/app/views/compare/_head.html.haml +++ b/app/views/compare/_head.html.haml @@ -19,5 +19,5 @@ - if current_page?(project_commits_path(@project)) && current_user.private_token %li.right %span.rss-icon - = link_to project_commits_path(@project, :atom, { private_token: current_user.private_token, ref: @ref }), title: "Feed" do + = link_to project_commits_path(@project, @ref, {format: :atom, private_token: current_user.private_token}), title: "Feed" do = image_tag "rss_ui.png", title: "feed" From f814da38bf3d86b8ff045ff585c75af4d6755184 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Tue, 25 Sep 2012 20:01:51 -0400 Subject: [PATCH 33/65] Limit commit history to 5 in Spinach Speeds things up a bit --- features/steps/shared/paths.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/features/steps/shared/paths.rb b/features/steps/shared/paths.rb index e1e1a718..fc23a798 100644 --- a/features/steps/shared/paths.rb +++ b/features/steps/shared/paths.rb @@ -47,7 +47,11 @@ module SharedPaths end Given 'I visit project commits page' do - visit project_commits_path(@project, @project.root_ref) + visit project_commits_path(@project, @project.root_ref, {limit: 5}) + end + + Given 'I visit project commits page for stable branch' do + visit project_commits_path(@project, 'stable', {limit: 5}) end Given 'I visit project source page' do From cada511f8b88671bb8a99600ed314c3beb5a36f4 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Tue, 25 Sep 2012 20:11:57 -0400 Subject: [PATCH 34/65] Add features for checking the "Active Tab" across various pages --- features/admin/active_tab.feature | 33 ++++ features/dashboard/active_tab.feature | 28 ++++ features/profile/active_tab.feature | 28 ++++ features/project/active_tab.feature | 147 ++++++++++++++++++ features/steps/admin/admin_active_tab.rb | 29 ++++ .../steps/dashboard/dashboard_active_tab.rb | 25 +++ features/steps/profile/profile_active_tab.rb | 25 +++ features/steps/project/project_active_tab.rb | 146 +++++++++++++++++ features/steps/shared/active_tab.rb | 11 ++ features/steps/shared/authentication.rb | 4 + features/steps/shared/paths.rb | 70 +++++++-- 11 files changed, 537 insertions(+), 9 deletions(-) create mode 100644 features/admin/active_tab.feature create mode 100644 features/dashboard/active_tab.feature create mode 100644 features/profile/active_tab.feature create mode 100644 features/project/active_tab.feature create mode 100644 features/steps/admin/admin_active_tab.rb create mode 100644 features/steps/dashboard/dashboard_active_tab.rb create mode 100644 features/steps/profile/profile_active_tab.rb create mode 100644 features/steps/project/project_active_tab.rb create mode 100644 features/steps/shared/active_tab.rb diff --git a/features/admin/active_tab.feature b/features/admin/active_tab.feature new file mode 100644 index 00000000..fce85ce9 --- /dev/null +++ b/features/admin/active_tab.feature @@ -0,0 +1,33 @@ +Feature: Admin active tab + Background: + Given I sign in as an admin + + Scenario: On Admin Home + Given I visit admin page + Then the active main tab should be Home + And no other main tabs should be active + + Scenario: On Admin Projects + Given I visit admin projects page + Then the active main tab should be Projects + And no other main tabs should be active + + Scenario: On Admin Users + Given I visit admin users page + Then the active main tab should be Users + And no other main tabs should be active + + Scenario: On Admin Logs + Given I visit admin logs page + Then the active main tab should be Logs + And no other main tabs should be active + + Scenario: On Admin Hooks + Given I visit admin hooks page + Then the active main tab should be Hooks + And no other main tabs should be active + + Scenario: On Admin Resque + Given I visit admin Resque page + Then the active main tab should be Resque + And no other main tabs should be active diff --git a/features/dashboard/active_tab.feature b/features/dashboard/active_tab.feature new file mode 100644 index 00000000..6715ea26 --- /dev/null +++ b/features/dashboard/active_tab.feature @@ -0,0 +1,28 @@ +Feature: Dashboard active tab + Background: + Given I sign in as a user + + Scenario: On Dashboard Home + Given I visit dashboard page + Then the active main tab should be Home + And no other main tabs should be active + + Scenario: On Dashboard Issues + Given I visit dashboard issues page + Then the active main tab should be Issues + And no other main tabs should be active + + Scenario: On Dashboard Merge Requests + Given I visit dashboard merge requests page + Then the active main tab should be Merge Requests + And no other main tabs should be active + + Scenario: On Dashboard Search + Given I visit dashboard search page + Then the active main tab should be Search + And no other main tabs should be active + + Scenario: On Dashboard Help + Given I visit dashboard help page + Then the active main tab should be Help + And no other main tabs should be active diff --git a/features/profile/active_tab.feature b/features/profile/active_tab.feature new file mode 100644 index 00000000..475641a3 --- /dev/null +++ b/features/profile/active_tab.feature @@ -0,0 +1,28 @@ +Feature: Profile active tab + Background: + Given I sign in as a user + + Scenario: On Profile Home + Given I visit profile page + Then the active main tab should be Home + And no other main tabs should be active + + Scenario: On Profile Account + Given I visit profile account page + Then the active main tab should be Account + And no other main tabs should be active + + Scenario: On Profile SSH Keys + Given I visit profile SSH keys page + Then the active main tab should be SSH Keys + And no other main tabs should be active + + Scenario: On Profile Design + Given I visit profile design page + Then the active main tab should be Design + And no other main tabs should be active + + Scenario: On Profile History + Given I visit profile history page + Then the active main tab should be History + And no other main tabs should be active diff --git a/features/project/active_tab.feature b/features/project/active_tab.feature new file mode 100644 index 00000000..2d3e41d3 --- /dev/null +++ b/features/project/active_tab.feature @@ -0,0 +1,147 @@ +Feature: Project active tab + Background: + Given I sign in as a user + And I own a project + + # Main Tabs + + Scenario: On Project Home + Given I visit my project's home page + Then the active main tab should be Home + And no other main tabs should be active + + Scenario: On Project Files + Given I visit my project's files page + Then the active main tab should be Files + And no other main tabs should be active + + Scenario: On Project Commits + Given I visit my project's commits page + Then the active main tab should be Commits + And no other main tabs should be active + + Scenario: On Project Network + Given I visit my project's network page + Then the active main tab should be Network + And no other main tabs should be active + + Scenario: On Project Issues + Given I visit my project's issues page + Then the active main tab should be Issues + And no other main tabs should be active + + Scenario: On Project Merge Requests + Given I visit my project's merge requests page + Then the active main tab should be Merge Requests + And no other main tabs should be active + + Scenario: On Project Wall + Given I visit my project's wall page + Then the active main tab should be Wall + And no other main tabs should be active + + Scenario: On Project Wiki + Given I visit my project's wiki page + Then the active main tab should be Wiki + And no other main tabs should be active + + # Sub Tabs: Home + + Scenario: On Project Home/Show + Given I visit my project's home page + Then the active sub tab should be Show + And no other sub tabs should be active + And the active main tab should be Home + + Scenario: On Project Home/Team + Given I visit my project's home page + And I click the "Team" tab + Then the active sub tab should be Team + And no other sub tabs should be active + And the active main tab should be Home + + Scenario: On Project Home/Attachments + Given I visit my project's home page + And I click the "Attachments" tab + Then the active sub tab should be Attachments + And no other sub tabs should be active + And the active main tab should be Home + + Scenario: On Project Home/Snippets + Given I visit my project's home page + And I click the "Snippets" tab + Then the active sub tab should be Snippets + And no other sub tabs should be active + And the active main tab should be Home + + Scenario: On Project Home/Edit + Given I visit my project's home page + And I click the "Edit" tab + Then the active sub tab should be Edit + And no other sub tabs should be active + And the active main tab should be Home + + Scenario: On Project Home/Hooks + Given I visit my project's home page + And I click the "Hooks" tab + Then the active sub tab should be Hooks + And no other sub tabs should be active + And the active main tab should be Home + + Scenario: On Project Home/Deploy Keys + Given I visit my project's home page + And I click the "Deploy Keys" tab + Then the active sub tab should be Deploy Keys + And no other sub tabs should be active + And the active main tab should be Home + + # Sub Tabs: Commits + + Scenario: On Project Commits/Commits + Given I visit my project's commits page + Then the active sub tab should be Commits + And no other sub tabs should be active + And the active main tab should be Commits + + Scenario: On Project Commits/Compare + Given I visit my project's commits page + And I click the "Compare" tab + Then the active sub tab should be Compare + And no other sub tabs should be active + And the active main tab should be Commits + + Scenario: On Project Commits/Branches + Given I visit my project's commits page + And I click the "Branches" tab + Then the active sub tab should be Branches + And no other sub tabs should be active + And the active main tab should be Commits + + Scenario: On Project Commits/Tags + Given I visit my project's commits page + And I click the "Tags" tab + Then the active sub tab should be Tags + And no other sub tabs should be active + And the active main tab should be Commits + + # Sub Tabs: Issues + + Scenario: On Project Issues/Browse + Given I visit my project's issues page + Then the active sub tab should be Browse Issues + And no other sub tabs should be active + And the active main tab should be Issues + + Scenario: On Project Issues/Milestones + Given I visit my project's issues page + And I click the "Milestones" tab + Then the active sub tab should be Milestones + And no other sub tabs should be active + And the active main tab should be Issues + + Scenario: On Project Issues/Labels + Given I visit my project's issues page + And I click the "Labels" tab + Then the active sub tab should be Labels + And no other sub tabs should be active + And the active main tab should be Issues diff --git a/features/steps/admin/admin_active_tab.rb b/features/steps/admin/admin_active_tab.rb new file mode 100644 index 00000000..29290892 --- /dev/null +++ b/features/steps/admin/admin_active_tab.rb @@ -0,0 +1,29 @@ +class AdminActiveTab < Spinach::FeatureSteps + include SharedAuthentication + include SharedPaths + include SharedActiveTab + + Then 'the active main tab should be Home' do + ensure_active_main_tab('Stats') + end + + Then 'the active main tab should be Projects' do + ensure_active_main_tab('Projects') + end + + Then 'the active main tab should be Users' do + ensure_active_main_tab('Users') + end + + Then 'the active main tab should be Logs' do + ensure_active_main_tab('Logs') + end + + Then 'the active main tab should be Hooks' do + ensure_active_main_tab('Hooks') + end + + Then 'the active main tab should be Resque' do + ensure_active_main_tab('Resque') + end +end diff --git a/features/steps/dashboard/dashboard_active_tab.rb b/features/steps/dashboard/dashboard_active_tab.rb new file mode 100644 index 00000000..41ecc48c --- /dev/null +++ b/features/steps/dashboard/dashboard_active_tab.rb @@ -0,0 +1,25 @@ +class DashboardActiveTab < Spinach::FeatureSteps + include SharedAuthentication + include SharedPaths + include SharedActiveTab + + Then 'the active main tab should be Home' do + ensure_active_main_tab('Home') + end + + Then 'the active main tab should be Issues' do + ensure_active_main_tab('Issues') + end + + Then 'the active main tab should be Merge Requests' do + ensure_active_main_tab('Merge Requests') + end + + Then 'the active main tab should be Search' do + ensure_active_main_tab('Search') + end + + Then 'the active main tab should be Help' do + ensure_active_main_tab('Help') + end +end diff --git a/features/steps/profile/profile_active_tab.rb b/features/steps/profile/profile_active_tab.rb new file mode 100644 index 00000000..1924a6fa --- /dev/null +++ b/features/steps/profile/profile_active_tab.rb @@ -0,0 +1,25 @@ +class ProfileActiveTab < Spinach::FeatureSteps + include SharedAuthentication + include SharedPaths + include SharedActiveTab + + Then 'the active main tab should be Home' do + ensure_active_main_tab('Profile') + end + + Then 'the active main tab should be Account' do + ensure_active_main_tab('Account') + end + + Then 'the active main tab should be SSH Keys' do + ensure_active_main_tab('SSH Keys') + end + + Then 'the active main tab should be Design' do + ensure_active_main_tab('Design') + end + + Then 'the active main tab should be History' do + ensure_active_main_tab('History') + end +end diff --git a/features/steps/project/project_active_tab.rb b/features/steps/project/project_active_tab.rb new file mode 100644 index 00000000..a5c80353 --- /dev/null +++ b/features/steps/project/project_active_tab.rb @@ -0,0 +1,146 @@ +class ProjectActiveTab < Spinach::FeatureSteps + include SharedAuthentication + include SharedPaths + include SharedProject + include SharedActiveTab + + # Main Tabs + + Then 'the active main tab should be Home' do + ensure_active_main_tab(@project.name) + end + + Then 'the active main tab should be Files' do + ensure_active_main_tab('Files') + end + + Then 'the active main tab should be Commits' do + ensure_active_main_tab('Commits') + end + + Then 'the active main tab should be Network' do + ensure_active_main_tab('Network') + end + + Then 'the active main tab should be Issues' do + ensure_active_main_tab('Issues') + end + + Then 'the active main tab should be Merge Requests' do + ensure_active_main_tab('Merge Requests') + end + + Then 'the active main tab should be Wall' do + ensure_active_main_tab('Wall') + end + + Then 'the active main tab should be Wiki' do + ensure_active_main_tab('Wiki') + end + + # Sub Tabs: Home + + Given 'I click the "Team" tab' do + click_link('Team') + end + + Given 'I click the "Attachments" tab' do + click_link('Attachments') + end + + Given 'I click the "Snippets" tab' do + click_link('Snippets') + end + + Given 'I click the "Edit" tab' do + click_link('Edit') + end + + Given 'I click the "Hooks" tab' do + click_link('Hooks') + end + + Given 'I click the "Deploy Keys" tab' do + click_link('Deploy Keys') + end + + Then 'the active sub tab should be Show' do + ensure_active_sub_tab('Show') + end + + Then 'the active sub tab should be Team' do + ensure_active_sub_tab('Team') + end + + Then 'the active sub tab should be Attachments' do + ensure_active_sub_tab('Attachments') + end + + Then 'the active sub tab should be Snippets' do + ensure_active_sub_tab('Snippets') + end + + Then 'the active sub tab should be Edit' do + ensure_active_sub_tab('Edit') + end + + Then 'the active sub tab should be Hooks' do + ensure_active_sub_tab('Hooks') + end + + Then 'the active sub tab should be Deploy Keys' do + ensure_active_sub_tab('Deploy Keys') + end + + # Sub Tabs: Commits + + Given 'I click the "Compare" tab' do + click_link('Compare') + end + + Given 'I click the "Branches" tab' do + click_link('Branches') + end + + Given 'I click the "Tags" tab' do + click_link('Tags') + end + + Then 'the active sub tab should be Commits' do + ensure_active_sub_tab('Commits') + end + + Then 'the active sub tab should be Compare' do + ensure_active_sub_tab('Compare') + end + + Then 'the active sub tab should be Branches' do + ensure_active_sub_tab('Branches') + end + + Then 'the active sub tab should be Tags' do + ensure_active_sub_tab('Tags') + end + + # Sub Tabs: Issues + + Given 'I click the "Milestones" tab' do + click_link('Milestones') + end + + Given 'I click the "Labels" tab' do + click_link('Labels') + end + + Then 'the active sub tab should be Browse Issues' do + ensure_active_sub_tab('Browse Issues') + end + + Then 'the active sub tab should be Milestones' do + ensure_active_sub_tab('Milestones') + end + + Then 'the active sub tab should be Labels' do + ensure_active_sub_tab('Labels') + end +end diff --git a/features/steps/shared/active_tab.rb b/features/steps/shared/active_tab.rb new file mode 100644 index 00000000..33ac5cf5 --- /dev/null +++ b/features/steps/shared/active_tab.rb @@ -0,0 +1,11 @@ +module SharedActiveTab + include Spinach::DSL + + def ensure_active_main_tab(content) + page.find('ul.main_menu li.current').should have_content(content) + end + + And 'no other main tabs should be active' do + page.should have_selector('ul.main_menu li.current', count: 1) + end +end diff --git a/features/steps/shared/authentication.rb b/features/steps/shared/authentication.rb index 77d9839f..2add9161 100644 --- a/features/steps/shared/authentication.rb +++ b/features/steps/shared/authentication.rb @@ -7,4 +7,8 @@ module SharedAuthentication Given 'I sign in as a user' do login_as :user end + + Given 'I sign in as an admin' do + login_as :admin + end end diff --git a/features/steps/shared/paths.rb b/features/steps/shared/paths.rb index fc23a798..bb35b8b0 100644 --- a/features/steps/shared/paths.rb +++ b/features/steps/shared/paths.rb @@ -1,22 +1,34 @@ module SharedPaths include Spinach::DSL - And 'I visit dashboard search page' do - visit search_path + # ---------------------------------------- + # Dashboard + # ---------------------------------------- + + Given 'I visit dashboard page' do + visit dashboard_path end - And 'I visit dashboard merge requests page' do - visit dashboard_merge_requests_path - end - - And 'I visit dashboard issues page' do + Given 'I visit dashboard issues page' do visit dashboard_issues_path end - When 'I visit dashboard page' do - visit dashboard_path + Given 'I visit dashboard merge requests page' do + visit dashboard_merge_requests_path end + Given 'I visit dashboard search page' do + visit search_path + end + + Given 'I visit dashboard help page' do + visit help_path + end + + # ---------------------------------------- + # Profile + # ---------------------------------------- + Given 'I visit profile page' do visit profile_path end @@ -25,10 +37,50 @@ module SharedPaths visit profile_account_path end + Given 'I visit profile SSH keys page' do + visit keys_path + end + + Given 'I visit profile design page' do + visit profile_design_path + end + + Given 'I visit profile history page' do + visit profile_history_path + end + Given 'I visit profile token page' do visit profile_token_path end + # ---------------------------------------- + # Admin + # ---------------------------------------- + + Given 'I visit admin page' do + visit admin_root_path + end + + Given 'I visit admin projects page' do + visit admin_projects_path + end + + Given 'I visit admin users page' do + visit admin_users_path + end + + Given 'I visit admin logs page' do + visit admin_logs_path + end + + Given 'I visit admin hooks page' do + visit admin_hooks_path + end + + Given 'I visit admin Resque page' do + visit admin_resque_path + end + When 'I visit new project page' do visit new_project_path end From 60ac6a28a2f198427c2d1ad68421aee484e14028 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Tue, 25 Sep 2012 21:18:00 -0400 Subject: [PATCH 35/65] Allow current_controller? helper to take an Array of options --- app/helpers/application_helper.rb | 11 +++++++---- spec/helpers/application_helper_spec.rb | 5 +++++ 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 505f6067..d916d887 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -3,13 +3,16 @@ module ApplicationHelper # Check if a particular controller is the current one # + # args - One or more controller names to check + # # Examples # # # On TreeController - # current_controller?(:tree) # => true - # current_controller?(:commits) # => false - def current_controller?(name) - controller.controller_name == name.to_s.downcase + # current_controller?(:tree) # => true + # current_controller?(:commits) # => false + # current_controller?(:commits, :tree) # => true + def current_controller?(*args) + args.any? { |v| v.to_s.downcase == controller.controller_name } end def gravatar_icon(user_email = '', size = 40) diff --git a/spec/helpers/application_helper_spec.rb b/spec/helpers/application_helper_spec.rb index 10250c93..fb711dd8 100644 --- a/spec/helpers/application_helper_spec.rb +++ b/spec/helpers/application_helper_spec.rb @@ -13,6 +13,11 @@ describe ApplicationHelper do it "returns false when controller does not match argument" do current_controller?(:bar).should_not be_true end + + it "should take any number of arguments" do + current_controller?(:baz, :bar).should_not be_true + current_controller?(:baz, :bar, :foo).should be_true + end end describe "gravatar_icon" do From adcc6a0b0e08158353627a8a900971aca07429bd Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Tue, 25 Sep 2012 21:18:39 -0400 Subject: [PATCH 36/65] Move tab_class helper to TabHelper --- app/helpers/application_helper.rb | 39 ------------------------------- app/helpers/tab_helper.rb | 39 +++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 39 deletions(-) diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index d916d887..f874851a 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -99,45 +99,6 @@ module ApplicationHelper event.project.merge_requests_enabled end - def tab_class(tab_key) - active = case tab_key - - # Project Area - when :wall; wall_tab? - when :wiki; controller.controller_name == "wikis" - when :issues; issues_tab? - when :network; current_page?(controller: "projects", action: "graph", id: @project) - when :merge_requests; controller.controller_name == "merge_requests" - - # Dashboard Area - when :help; controller.controller_name == "help" - when :search; current_page?(search_path) - when :dash_issues; current_page?(dashboard_issues_path) - when :dash_mr; current_page?(dashboard_merge_requests_path) - when :root; current_page?(dashboard_path) || current_page?(root_path) - - # Profile Area - when :profile; current_page?(controller: "profile", action: :show) - when :history; current_page?(controller: "profile", action: :history) - when :account; current_page?(controller: "profile", action: :account) - when :token; current_page?(controller: "profile", action: :token) - when :design; current_page?(controller: "profile", action: :design) - when :ssh_keys; controller.controller_name == "keys" - - # Admin Area - when :admin_root; controller.controller_name == "dashboard" - when :admin_users; controller.controller_name == 'users' - when :admin_projects; controller.controller_name == "projects" - when :admin_hooks; controller.controller_name == 'hooks' - when :admin_resque; controller.controller_name == 'resque' - when :admin_logs; controller.controller_name == 'logs' - - else - false - end - active ? "current" : nil - end - def hexdigest(string) Digest::SHA1.hexdigest string end diff --git a/app/helpers/tab_helper.rb b/app/helpers/tab_helper.rb index b5d7ccb7..4cc97b11 100644 --- a/app/helpers/tab_helper.rb +++ b/app/helpers/tab_helper.rb @@ -1,4 +1,43 @@ module TabHelper + def tab_class(tab_key) + active = case tab_key + + # Project Area + when :wall; wall_tab? + when :wiki; controller.controller_name == "wikis" + when :issues; issues_tab? + when :network; current_page?(controller: "projects", action: "graph", id: @project) + when :merge_requests; controller.controller_name == "merge_requests" + + # Dashboard Area + when :help; controller.controller_name == "help" + when :search; current_page?(search_path) + when :dash_issues; current_page?(dashboard_issues_path) + when :dash_mr; current_page?(dashboard_merge_requests_path) + when :root; current_page?(dashboard_path) || current_page?(root_path) + + # Profile Area + when :profile; current_page?(controller: "profile", action: :show) + when :history; current_page?(controller: "profile", action: :history) + when :account; current_page?(controller: "profile", action: :account) + when :token; current_page?(controller: "profile", action: :token) + when :design; current_page?(controller: "profile", action: :design) + when :ssh_keys; controller.controller_name == "keys" + + # Admin Area + when :admin_root; controller.controller_name == "dashboard" + when :admin_users; controller.controller_name == 'users' + when :admin_projects; controller.controller_name == "projects" + when :admin_hooks; controller.controller_name == 'hooks' + when :admin_resque; controller.controller_name == 'resque' + when :admin_logs; controller.controller_name == 'logs' + + else + false + end + active ? "current" : nil + end + def issues_tab? controller.controller_name == "issues" || controller.controller_name == "milestones" end From 51c1e499006df02ff3dfc2a781457a01cc77550f Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Tue, 25 Sep 2012 22:07:41 -0400 Subject: [PATCH 37/65] Change active tab and nav class to "active" The main nav used active, the sub nav used current. This normalizes it. --- app/assets/stylesheets/sections/nav.scss | 2 +- app/helpers/tab_helper.rb | 17 ++++------------- app/views/layouts/_project_menu.html.haml | 8 +++----- features/steps/shared/active_tab.rb | 12 ++++++++++-- 4 files changed, 18 insertions(+), 21 deletions(-) diff --git a/app/assets/stylesheets/sections/nav.scss b/app/assets/stylesheets/sections/nav.scss index 2d902918..98bc7275 100644 --- a/app/assets/stylesheets/sections/nav.scss +++ b/app/assets/stylesheets/sections/nav.scss @@ -53,7 +53,7 @@ ul.main_menu { border-left: 0; } - &.current { + &.active { background-color:#D5D5D5; border-right: 1px solid #BBB; border-left: 1px solid #BBB; diff --git a/app/helpers/tab_helper.rb b/app/helpers/tab_helper.rb index 4cc97b11..9491e277 100644 --- a/app/helpers/tab_helper.rb +++ b/app/helpers/tab_helper.rb @@ -5,7 +5,6 @@ module TabHelper # Project Area when :wall; wall_tab? when :wiki; controller.controller_name == "wikis" - when :issues; issues_tab? when :network; current_page?(controller: "projects", action: "graph", id: @project) when :merge_requests; controller.controller_name == "merge_requests" @@ -35,11 +34,7 @@ module TabHelper else false end - active ? "current" : nil - end - - def issues_tab? - controller.controller_name == "issues" || controller.controller_name == "milestones" + active ? "active" : nil end def wall_tab? @@ -48,21 +43,17 @@ module TabHelper def project_tab_class [:show, :files, :edit, :update].each do |action| - return "current" if current_page?(controller: "projects", action: action, id: @project) + return "active" if current_page?(controller: "projects", action: action, id: @project) end if ['snippets', 'hooks', 'deploy_keys', 'team_members'].include? controller.controller_name - "current" + "active" end end - def tree_tab_class - controller.controller_name == "refs" ? "current" : nil - end - def commit_tab_class if ['commits', 'repositories', 'protected_branches'].include? controller.controller_name - "current" + "active" end end diff --git a/app/views/layouts/_project_menu.html.haml b/app/views/layouts/_project_menu.html.haml index 13a4d037..e2575646 100644 --- a/app/views/layouts/_project_menu.html.haml +++ b/app/views/layouts/_project_menu.html.haml @@ -4,17 +4,15 @@ - if @project.repo_exists? - if can? current_user, :download_code, @project - %li{class: tree_tab_class} - = link_to project_tree_path(@project, @project.root_ref) do - Files + %li{class: current_controller?(:tree, :blob, :blame) ? 'active' : ''} + = link_to 'Files', project_tree_path(@project, @project.root_ref) %li{class: commit_tab_class} = link_to "Commits", project_commits_path(@project, @project.root_ref) - %li{class: tab_class(:network)} = link_to "Network", graph_project_path(@project) - if @project.issues_enabled - %li{class: tab_class(:issues)} + %li{class: current_controller?(:issues, :milestones, :labels) ? 'active' : ''} = link_to project_issues_filter_path(@project) do Issues %span.count.issue_counter= @project.issues.opened.count diff --git a/features/steps/shared/active_tab.rb b/features/steps/shared/active_tab.rb index 33ac5cf5..884f2d5f 100644 --- a/features/steps/shared/active_tab.rb +++ b/features/steps/shared/active_tab.rb @@ -2,10 +2,18 @@ module SharedActiveTab include Spinach::DSL def ensure_active_main_tab(content) - page.find('ul.main_menu li.current').should have_content(content) + page.find('ul.main_menu li.active').should have_content(content) + end + + def ensure_active_sub_tab(content) + page.find('div.content ul.nav-tabs li.active').should have_content(content) end And 'no other main tabs should be active' do - page.should have_selector('ul.main_menu li.current', count: 1) + page.should have_selector('ul.main_menu li.active', count: 1) + end + + And 'no other sub tabs should be active' do + page.should have_selector('div.content ul.nav-tabs li.active', count: 1) end end From 6b90f36f5b6ee36dede7cc33c9364ddd34e5ba6b Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Tue, 25 Sep 2012 22:32:54 -0400 Subject: [PATCH 38/65] Updates to routing specs --- spec/routing/project_routing_spec.rb | 57 ++++++---------------------- 1 file changed, 11 insertions(+), 46 deletions(-) diff --git a/spec/routing/project_routing_spec.rb b/spec/routing/project_routing_spec.rb index 2939f2fc..652a75a4 100644 --- a/spec/routing/project_routing_spec.rb +++ b/spec/routing/project_routing_spec.rb @@ -285,6 +285,7 @@ end describe CommitController, "routing" do it "to #show" do get("/gitlabhq/commit/4246fb").should route_to('commit#show', project_id: 'gitlabhq', id: '4246fb') + get("/gitlabhq/commit/4246fb.patch").should route_to('commit#show', project_id: 'gitlabhq', id: '4246fb', format: 'patch') get("/gitlabhq/commit/4246fbd13872934f72a8fd0d6fb1317b47b59cb5").should route_to('commit#show', project_id: 'gitlabhq', id: '4246fbd13872934f72a8fd0d6fb1317b47b59cb5') end end @@ -294,16 +295,8 @@ end # POST /:project_id/commits(.:format) commits#create # project_commit GET /:project_id/commits/:id(.:format) commits#show describe CommitsController, "routing" do - it "to #patch" do - get("/gitlabhq/commits/1/patch").should route_to('commits#patch', project_id: 'gitlabhq', id: '1') - end - - it "does something with atom feeds" do - get("/gitlabhq/commits/master.atom").should route_to('commits#show', project_id: 'gitlabhq', id: 'master.atom') - end - it_behaves_like "RESTful project resources" do - let(:actions) { [:index, :show] } + let(:actions) { [:show] } let(:controller) { 'commits' } end end @@ -384,64 +377,36 @@ describe NotesController, "routing" do end end +# project_blame GET /:project_id/blame/:id(.:format) blame#show {:id=>/.+/, :project_id=>/[^\/]+/} describe BlameController, "routing" do it "to #show" do get("/gitlabhq/blame/master/app/models/project.rb").should route_to('blame#show', project_id: 'gitlabhq', id: 'master/app/models/project.rb') end end +# project_blob GET /:project_id/blob/:id(.:format) blob#show {:id=>/.+/, :project_id=>/[^\/]+/} describe BlobController, "routing" do it "to #show" do get("/gitlabhq/blob/master/app/models/project.rb").should route_to('blob#show', project_id: 'gitlabhq', id: 'master/app/models/project.rb') end end +# project_tree GET /:project_id/tree/:id(.:format) tree#show {:id=>/.+/, :project_id=>/[^\/]+/} describe TreeController, "routing" do it "to #show" do get("/gitlabhq/tree/master/app/models/project.rb").should route_to('tree#show', project_id: 'gitlabhq', id: 'master/app/models/project.rb') end end +# project_compare_index GET /:project_id/compare(.:format) compare#index {:id=>/[^\/]+/, :project_id=>/[^\/]+/} +# project_compare /:project_id/compare/:from...:to(.:format) compare#show {:from=>/.+/, :to=>/.+/, :id=>/[^\/]+/, :project_id=>/[^\/]+/} describe CompareController, "routing" do + it "to #index" do + get("/gitlabhq/compare").should route_to('compare#index', project_id: 'gitlabhq') + end + it "to #show" do get("/gitlabhq/compare/master...stable").should route_to('compare#show', project_id: 'gitlabhq', from: 'master', to: 'stable') get("/gitlabhq/compare/issue/1234...stable").should route_to('compare#show', project_id: 'gitlabhq', from: 'issue/1234', to: 'stable') end end - -# TODO: Pending -# -# /:project_id/blame/*path -# /gitlabhq/blame/master/app/contexts/base_context.rb -# /gitlabhq/blame/test/branch/name/app/contexts/base_context.rb -# -# /:project_id/blob/*path -# /gitlabhq/blob/master/app/contexts/base_context.rb -# /gitlabhq/blob/test/branch/name/app/contexts/base_context.rb -# -# /:project_id/commit/:id -# /gitlabhq/commit/caef9ed1121a16ca0cc78715695daaa974271bfd -# -# /:project_id/commits -# -# /:project_id/commits/*path -# /gitlabhq/commits/master/app/contexts/base_context.rb -# /gitlabhq/commits/test/branch/name/app/contexts/base_context.rb -# /gitlabhq/commits/master.atom -# -# /:project_id/raw/*path -# /gitlabhq/raw/master/app/contexts/base_context.rb -# /gitlabhq/raw/test/branch/name/app/contexts/base_context.rb -# -# /:project_id/tree/*path -# /gitlabhq/tree/master/app -# /gitlabhq/tree/test/branch/name/app -describe "pending routing" do - before { pending } - - describe "/:project_id/raw/:id" do - it "routes to a ref with a path" do - get("/gitlabhq/raw/master/app/models/project.rb").should route_to('raw#show', project_id: 'gitlabhq', id: 'master/app/models/project.rb') - end - end -end From 4252ea90c0ab757fce538af1f03ce5f6d48f7814 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Tue, 25 Sep 2012 22:34:29 -0400 Subject: [PATCH 39/65] Add feature steps for a generic, non-"Shop" project This group is better for features that only deal with one project. --- features/steps/shared/paths.rb | 48 ++++++++++++++++++++++++++++++-- features/steps/shared/project.rb | 7 +++++ features/support/env.rb | 2 ++ 3 files changed, 55 insertions(+), 2 deletions(-) diff --git a/features/steps/shared/paths.rb b/features/steps/shared/paths.rb index bb35b8b0..a1a8efd6 100644 --- a/features/steps/shared/paths.rb +++ b/features/steps/shared/paths.rb @@ -1,6 +1,10 @@ module SharedPaths include Spinach::DSL + When 'I visit new project page' do + visit new_project_path + end + # ---------------------------------------- # Dashboard # ---------------------------------------- @@ -81,10 +85,50 @@ module SharedPaths visit admin_resque_path end - When 'I visit new project page' do - visit new_project_path + # ---------------------------------------- + # Generic Project + # ---------------------------------------- + + Given "I visit my project's home page" do + visit project_path(@project) end + Given "I visit my project's files page" do + visit project_tree_path(@project, @project.root_ref) + end + + Given "I visit my project's commits page" do + visit project_commits_path(@project, @project.root_ref, {limit: 5}) + end + + Given "I visit my project's network page" do + # Stub out find_all to speed this up (10 commits vs. 650) + commits = Grit::Commit.find_all(@project.repo, nil, {max_count: 10}) + Grit::Commit.stub(:find_all).and_return(commits) + + visit graph_project_path(@project) + end + + Given "I visit my project's issues page" do + visit project_issues_path(@project) + end + + Given "I visit my project's merge requests page" do + visit project_merge_requests_path(@project) + end + + Given "I visit my project's wall page" do + visit wall_project_path(@project) + end + + Given "I visit my project's wiki page" do + visit project_wiki_path(@project, :index) + end + + # ---------------------------------------- + # "Shop" Project + # ---------------------------------------- + And 'I visit project "Shop" page' do project = Project.find_by_name("Shop") visit project_path(project) diff --git a/features/steps/shared/project.rb b/features/steps/shared/project.rb index 9b64ca59..0f93d675 100644 --- a/features/steps/shared/project.rb +++ b/features/steps/shared/project.rb @@ -1,6 +1,13 @@ module SharedProject include Spinach::DSL + # Create a project without caring about what it's called + And "I own a project" do + @project = create(:project) + @project.add_access(@user, :admin) + end + + # Create a specific project called "Shop" And 'I own project "Shop"' do @project = Factory :project, :name => "Shop" @project.add_access(@user, :admin) diff --git a/features/support/env.rb b/features/support/env.rb index 9c6cef07..6d49c25a 100644 --- a/features/support/env.rb +++ b/features/support/env.rb @@ -23,5 +23,7 @@ Spinach.hooks.after_scenario { DatabaseCleaner.clean } Spinach.hooks.before_run do RSpec::Mocks::setup self + include FactoryGirl::Syntax::Methods + stub_gitolite! end From 8fe63dab52f6e72aaab1141f2af5d72b012064b7 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Tue, 25 Sep 2012 22:41:25 -0400 Subject: [PATCH 40/65] Use current_controller? in layouts/_head partial --- app/views/layouts/_head.html.haml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/views/layouts/_head.html.haml b/app/views/layouts/_head.html.haml index 1674be27..c130c565 100644 --- a/app/views/layouts/_head.html.haml +++ b/app/views/layouts/_head.html.haml @@ -10,8 +10,8 @@ - if controller_name == 'projects' && action_name == 'index' = auto_discovery_link_tag :atom, projects_url(:atom, private_token: current_user.private_token), title: "Dashboard feed" - if @project && !@project.new_record? - - if current_page?(project_tree_path(@project, @ref)) || current_page?(project_commits_path(@project, @ref)) + - if current_controller?(:tree, :commit, :commits) = auto_discovery_link_tag(:atom, project_commits_url(@project, @ref, format: :atom, private_token: current_user.private_token), title: "Recent commits to #{@project.name}:#{@ref}") - - if request.path == project_issues_path(@project) + - if current_controller?(:issues) = auto_discovery_link_tag(:atom, project_issues_url(@project, :atom, private_token: current_user.private_token), title: "#{@project.name} issues") = csrf_meta_tags From 6cb626ef516a230b2af4e15145488d4c57cd048c Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Tue, 25 Sep 2012 23:10:50 -0400 Subject: [PATCH 41/65] Add Compare#index and Compare#create actions Create just redirects to our specially-formatted #show action --- app/controllers/compare_controller.rb | 7 +++++++ config/routes.rb | 2 +- spec/routing/project_routing_spec.rb | 5 +++++ 3 files changed, 13 insertions(+), 1 deletion(-) diff --git a/app/controllers/compare_controller.rb b/app/controllers/compare_controller.rb index 1124fd0c..62f968fd 100644 --- a/app/controllers/compare_controller.rb +++ b/app/controllers/compare_controller.rb @@ -8,6 +8,9 @@ class CompareController < ApplicationController before_filter :authorize_code_access! before_filter :require_non_empty_project + def index + end + def show result = Commit.compare(project, params[:from], params[:to]) @@ -19,4 +22,8 @@ class CompareController < ApplicationController @commits = CommitDecorator.decorate(@commits) end + + def create + redirect_to project_compare_path(@project, params[:from], params[:to]) + end end diff --git a/config/routes.rb b/config/routes.rb index bc29314e..decf860b 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -162,7 +162,7 @@ Gitlab::Application.routes.draw do # XXX: WIP resources :commit, only: [:show], constraints: {id: /[[:alnum:]]{6,40}/} resources :commits, only: [:show], constraints: {id: /.+/} - resources :compare, only: [:index] + resources :compare, only: [:index, :create] resources :blame, only: [:show], constraints: {id: /.+/} resources :blob, only: [:show], constraints: {id: /.+/} resources :tree, only: [:show], constraints: {id: /.+/} diff --git a/spec/routing/project_routing_spec.rb b/spec/routing/project_routing_spec.rb index 652a75a4..dc687d2a 100644 --- a/spec/routing/project_routing_spec.rb +++ b/spec/routing/project_routing_spec.rb @@ -399,12 +399,17 @@ describe TreeController, "routing" do end # project_compare_index GET /:project_id/compare(.:format) compare#index {:id=>/[^\/]+/, :project_id=>/[^\/]+/} +# POST /:project_id/compare(.:format) compare#create {:id=>/[^\/]+/, :project_id=>/[^\/]+/} # project_compare /:project_id/compare/:from...:to(.:format) compare#show {:from=>/.+/, :to=>/.+/, :id=>/[^\/]+/, :project_id=>/[^\/]+/} describe CompareController, "routing" do it "to #index" do get("/gitlabhq/compare").should route_to('compare#index', project_id: 'gitlabhq') end + it "to #compare" do + post("/gitlabhq/compare").should route_to('compare#create', project_id: 'gitlabhq') + end + it "to #show" do get("/gitlabhq/compare/master...stable").should route_to('compare#show', project_id: 'gitlabhq', from: 'master', to: 'stable') get("/gitlabhq/compare/issue/1234...stable").should route_to('compare#show', project_id: 'gitlabhq', from: 'issue/1234', to: 'stable') From b462a133879ca2a202f18f18d8e78a3ffed05c24 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Tue, 25 Sep 2012 23:13:19 -0400 Subject: [PATCH 42/65] Compare views and cleanup - Remove compare/_head and just use commits/_head since they're identical - Add index view; extract the form into a partial --- app/views/commits/_head.html.haml | 4 +-- app/views/compare/_form.html.haml | 32 ++++++++++++++++++++ app/views/compare/_head.html.haml | 23 --------------- app/views/compare/index.html.haml | 7 +++++ app/views/compare/show.html.haml | 41 ++------------------------ app/views/events/_event_push.html.haml | 2 +- features/steps/shared/paths.rb | 2 +- 7 files changed, 45 insertions(+), 66 deletions(-) create mode 100644 app/views/compare/_form.html.haml delete mode 100644 app/views/compare/_head.html.haml create mode 100644 app/views/compare/index.html.haml diff --git a/app/views/commits/_head.html.haml b/app/views/commits/_head.html.haml index cdee391f..86a65f87 100644 --- a/app/views/commits/_head.html.haml +++ b/app/views/commits/_head.html.haml @@ -3,8 +3,8 @@ %li{class: "#{'active' if current_page?(project_commits_path(@project)) }"} = link_to project_commits_path(@project) do Commits - %li{class: "#{'active' if current_page?(compare_project_commits_path(@project)) }"} - = link_to compare_project_commits_path(@project) do + %li{class: "#{'active' if current_controller?(:compare)}"} + = link_to project_compare_index_path(@project) do Compare %li{class: "#{branches_tab_class}"} = link_to project_repository_path(@project) do diff --git a/app/views/compare/_form.html.haml b/app/views/compare/_form.html.haml new file mode 100644 index 00000000..07f1c818 --- /dev/null +++ b/app/views/compare/_form.html.haml @@ -0,0 +1,32 @@ +%div + %p.slead + Fill input field with commit id like + %code.label_branch 4eedf23 + or branch/tag name like + %code.label_branch master + and press compare button for commits list, code diff. + + %br + + = form_tag project_compare_index_path(@project), method: :post do + .clearfix + = text_field_tag :from, params[:from], placeholder: "master", class: "xlarge" + = "..." + = text_field_tag :to, params[:to], placeholder: "aa8b4ef", class: "xlarge" + - if @refs_are_same + .alert + %span Refs are the same + .actions + = submit_tag "Compare", class: "btn primary wide commits-compare-btn" + +:javascript + $(function() { + var availableTags = #{@project.ref_names.to_json}; + + $("#from, #to").autocomplete({ + source: availableTags, + minLength: 1 + }); + + disableButtonIfEmptyField('#to', '.commits-compare-btn'); + }); diff --git a/app/views/compare/_head.html.haml b/app/views/compare/_head.html.haml deleted file mode 100644 index cdee391f..00000000 --- a/app/views/compare/_head.html.haml +++ /dev/null @@ -1,23 +0,0 @@ -%ul.nav.nav-tabs - %li= render partial: 'shared/ref_switcher', locals: {destination: 'commits'} - %li{class: "#{'active' if current_page?(project_commits_path(@project)) }"} - = link_to project_commits_path(@project) do - Commits - %li{class: "#{'active' if current_page?(compare_project_commits_path(@project)) }"} - = link_to compare_project_commits_path(@project) do - Compare - %li{class: "#{branches_tab_class}"} - = link_to project_repository_path(@project) do - Branches - %span.badge= @project.repo.branch_count - - %li{class: "#{'active' if current_page?(tags_project_repository_path(@project)) }"} - = link_to tags_project_repository_path(@project) do - Tags - %span.badge= @project.repo.tag_count - - - if current_page?(project_commits_path(@project)) && current_user.private_token - %li.right - %span.rss-icon - = link_to project_commits_path(@project, @ref, {format: :atom, private_token: current_user.private_token}), title: "Feed" do - = image_tag "rss_ui.png", title: "feed" diff --git a/app/views/compare/index.html.haml b/app/views/compare/index.html.haml new file mode 100644 index 00000000..6c9a5fd8 --- /dev/null +++ b/app/views/compare/index.html.haml @@ -0,0 +1,7 @@ += render "commits/head" + +%h3.page_title + Compare View +%hr + += render "form" diff --git a/app/views/compare/show.html.haml b/app/views/compare/show.html.haml index db15ba53..528c8b44 100644 --- a/app/views/compare/show.html.haml +++ b/app/views/compare/show.html.haml @@ -1,29 +1,10 @@ -= render "head" += render "commits/head" %h3.page_title Compare View %hr -%div - %p.slead - Fill input field with commit id like - %code.label_branch 4eedf23 - or branch/tag name like - %code.label_branch master - and press compare button for commits list, code diff. - - %br - - = form_tag compare_project_commits_path(@project), method: :get do - .clearfix - = text_field_tag :from, params[:from], placeholder: "master", class: "xlarge" - = "..." - = text_field_tag :to, params[:to], placeholder: "aa8b4ef", class: "xlarge" - - if @refs_are_same - .alert - %span Refs are the same - .actions - = submit_tag "Compare", class: "btn primary wide commits-compare-btn" += render "form" - if @commits.present? %div.ui-box @@ -33,21 +14,3 @@ - unless @diffs.empty? %h4 Diff = render "commits/diffs", diffs: @diffs - -:javascript - $(function() { - var availableTags = #{@project.ref_names.to_json}; - - $("#from").autocomplete({ - source: availableTags, - minLength: 1 - }); - - $("#to").autocomplete({ - source: availableTags, - minLength: 1 - }); - - disableButtonIfEmptyField('#to', '.commits-compare-btn'); - }); - diff --git a/app/views/events/_event_push.html.haml b/app/views/events/_event_push.html.haml index c0be9cf5..19ac9263 100644 --- a/app/views/events/_event_push.html.haml +++ b/app/views/events/_event_push.html.haml @@ -21,6 +21,6 @@ %li.commits-stat - if event.commits_count > 2 %span ... and #{event.commits_count - 2} more commits. - = link_to compare_project_commits_path(event.project, from: event.parent_commit.id, to: event.last_commit.id) do + = link_to project_compare_path(event.project, from: event.parent_commit.id, to: event.last_commit.id) do %strong Compare → #{event.parent_commit.id[0..7]}...#{event.last_commit.id[0..7]} .clearfix diff --git a/features/steps/shared/paths.rb b/features/steps/shared/paths.rb index a1a8efd6..8efedfa1 100644 --- a/features/steps/shared/paths.rb +++ b/features/steps/shared/paths.rb @@ -139,7 +139,7 @@ module SharedPaths end Given 'I visit compare refs page' do - visit compare_project_commits_path(@project) + visit project_compare_index_path(@project) end Given 'I visit project commits page' do From 86a7864dc78ed9168df889a5994ee7ef80a29476 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Tue, 25 Sep 2012 23:33:42 -0400 Subject: [PATCH 43/65] Fix various links --- app/views/blame/show.html.haml | 2 +- app/views/events/_event_last_push.html.haml | 2 +- app/views/events/_event_push.html.haml | 2 +- app/views/layouts/_head.html.haml | 2 +- app/views/protected_branches/index.html.haml | 2 +- app/views/repositories/_branch.html.haml | 2 +- app/views/repositories/_feed.html.haml | 2 +- app/views/repositories/tags.html.haml | 2 +- app/views/tree/_tree_file.html.haml | 2 +- 9 files changed, 9 insertions(+), 9 deletions(-) diff --git a/app/views/blame/show.html.haml b/app/views/blame/show.html.haml index 12b4e668..8f82b00f 100644 --- a/app/views/blame/show.html.haml +++ b/app/views/blame/show.html.haml @@ -19,7 +19,7 @@ %small blame %span.options = link_to "raw", project_blob_path(@project, @id), class: "btn very_small", target: "_blank" - = link_to "history", project_commits_path(@project, path: params[:path], ref: @ref), class: "btn very_small" + = link_to "history", project_commits_path(@project, @id), class: "btn very_small" = link_to "source", project_tree_path(@project, @id), class: "btn very_small" .file_content.blame %table diff --git a/app/views/events/_event_last_push.html.haml b/app/views/events/_event_last_push.html.haml index 81b9994c..d70be70c 100644 --- a/app/views/events/_event_last_push.html.haml +++ b/app/views/events/_event_last_push.html.haml @@ -4,7 +4,7 @@ = image_tag gravatar_icon(event.author_email), class: "avatar" %span You pushed to = event.ref_type - = link_to project_commits_path(event.project, ref: event.ref_name) do + = link_to project_commits_path(event.project, event.ref_name) do %strong= truncate(event.ref_name, length: 28) at %strong= link_to event.project.name, event.project diff --git a/app/views/events/_event_push.html.haml b/app/views/events/_event_push.html.haml index 19ac9263..9b7f10d9 100644 --- a/app/views/events/_event_push.html.haml +++ b/app/views/events/_event_push.html.haml @@ -5,7 +5,7 @@ .event-title %strong.author_name #{event.author_name} %span.event_label.pushed #{event.push_action_name} #{event.ref_type} - = link_to project_commits_path(event.project, ref: event.ref_name) do + = link_to project_commits_path(event.project, event.ref_name) do %strong= event.ref_name at %strong= link_to event.project.name, event.project diff --git a/app/views/layouts/_head.html.haml b/app/views/layouts/_head.html.haml index c130c565..25fe9d80 100644 --- a/app/views/layouts/_head.html.haml +++ b/app/views/layouts/_head.html.haml @@ -10,7 +10,7 @@ - if controller_name == 'projects' && action_name == 'index' = auto_discovery_link_tag :atom, projects_url(:atom, private_token: current_user.private_token), title: "Dashboard feed" - if @project && !@project.new_record? - - if current_controller?(:tree, :commit, :commits) + - if current_controller?(:tree, :commits) = auto_discovery_link_tag(:atom, project_commits_url(@project, @ref, format: :atom, private_token: current_user.private_token), title: "Recent commits to #{@project.name}:#{@ref}") - if current_controller?(:issues) = auto_discovery_link_tag(:atom, project_issues_url(@project, :atom, private_token: current_user.private_token), title: "#{@project.name} issues") diff --git a/app/views/protected_branches/index.html.haml b/app/views/protected_branches/index.html.haml index 43884de1..50a81712 100644 --- a/app/views/protected_branches/index.html.haml +++ b/app/views/protected_branches/index.html.haml @@ -34,7 +34,7 @@ - @branches.each do |branch| %tr %td - = link_to project_commits_path(@project, ref: branch.name) do + = link_to project_commits_path(@project, branch.name) do %strong= branch.name - if branch.name == @project.root_ref %span.label default diff --git a/app/views/repositories/_branch.html.haml b/app/views/repositories/_branch.html.haml index 80028a82..3c1fe47c 100644 --- a/app/views/repositories/_branch.html.haml +++ b/app/views/repositories/_branch.html.haml @@ -2,7 +2,7 @@ - commit = CommitDecorator.decorate(commit) %tr %td - = link_to project_commits_path(@project, ref: branch.name) do + = link_to project_commits_path(@project, branch.name) do %strong= truncate(branch.name, length: 60) - if branch.name == @project.root_ref %span.label default diff --git a/app/views/repositories/_feed.html.haml b/app/views/repositories/_feed.html.haml index 0c13551d..496328ba 100644 --- a/app/views/repositories/_feed.html.haml +++ b/app/views/repositories/_feed.html.haml @@ -2,7 +2,7 @@ - commit = CommitDecorator.new(commit) %tr %td - = link_to project_commits_path(@project, ref: commit.head.name) do + = link_to project_commits_path(@project, commit.head.name) do %strong = commit.head.name - if commit.head.name == @project.root_ref diff --git a/app/views/repositories/tags.html.haml b/app/views/repositories/tags.html.haml index a4114586..38cc3aca 100644 --- a/app/views/repositories/tags.html.haml +++ b/app/views/repositories/tags.html.haml @@ -12,7 +12,7 @@ - commit = CommitDecorator.decorate(commit) %tr %td - %strong= link_to tag.name, project_commits_path(@project, ref: tag.name), class: "" + %strong= link_to tag.name, project_commits_path(@project, tag.name), class: "" %td = link_to project_commit_path(@project, commit.id) do %code= commit.short_id diff --git a/app/views/tree/_tree_file.html.haml b/app/views/tree/_tree_file.html.haml index b4c4566b..93f7be28 100644 --- a/app/views/tree/_tree_file.html.haml +++ b/app/views/tree/_tree_file.html.haml @@ -6,7 +6,7 @@ %small #{file.mode} %span.options = link_to "raw", project_blob_path(@project, @id), class: "btn very_small", target: "_blank" - = link_to "history", project_commits_path(@project, path: @path, ref: @ref), class: "btn very_small" + = link_to "history", project_commits_path(@project, @id), class: "btn very_small" = link_to "blame", project_blame_path(@project, @id), class: "btn very_small" - if file.text? - if gitlab_markdown?(name) From b62d6a1fe28ad040e96fd88604131f357e5e6e9e Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Tue, 25 Sep 2012 23:34:26 -0400 Subject: [PATCH 44/65] Remove commit_tab_class helper --- app/helpers/tab_helper.rb | 6 ------ app/views/layouts/_project_menu.html.haml | 2 +- 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/app/helpers/tab_helper.rb b/app/helpers/tab_helper.rb index 9491e277..c26e3467 100644 --- a/app/helpers/tab_helper.rb +++ b/app/helpers/tab_helper.rb @@ -51,12 +51,6 @@ module TabHelper end end - def commit_tab_class - if ['commits', 'repositories', 'protected_branches'].include? controller.controller_name - "active" - end - end - def branches_tab_class if current_page?(branches_project_repository_path(@project)) || controller.controller_name == "protected_branches" || diff --git a/app/views/layouts/_project_menu.html.haml b/app/views/layouts/_project_menu.html.haml index e2575646..5a662a45 100644 --- a/app/views/layouts/_project_menu.html.haml +++ b/app/views/layouts/_project_menu.html.haml @@ -6,7 +6,7 @@ - if can? current_user, :download_code, @project %li{class: current_controller?(:tree, :blob, :blame) ? 'active' : ''} = link_to 'Files', project_tree_path(@project, @project.root_ref) - %li{class: commit_tab_class} + %li{class: current_controller?(:commit, :commits, :compare, :repositories, :protected_branches) ? 'active' : ''} = link_to "Commits", project_commits_path(@project, @project.root_ref) %li{class: tab_class(:network)} = link_to "Network", graph_project_path(@project) From b6d6663e0e793b2a47f8a25215334f77e2b80baf Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Tue, 25 Sep 2012 23:34:52 -0400 Subject: [PATCH 45/65] Fix the remaining actions in RefsController --- app/controllers/refs_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/refs_controller.rb b/app/controllers/refs_controller.rb index ff143f3f..6ae816ec 100644 --- a/app/controllers/refs_controller.rb +++ b/app/controllers/refs_controller.rb @@ -69,6 +69,6 @@ class RefsController < ApplicationController end def ref - @ref = params[:ref] + @ref = params[:id] || params[:ref] end end From 0887dda8e4eba68004e087e12302f3dbf2688790 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Tue, 25 Sep 2012 23:39:28 -0400 Subject: [PATCH 46/65] Use current ref for Files and Commits if available Closes #1452 --- app/views/layouts/_project_menu.html.haml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/views/layouts/_project_menu.html.haml b/app/views/layouts/_project_menu.html.haml index 5a662a45..d69f9a0b 100644 --- a/app/views/layouts/_project_menu.html.haml +++ b/app/views/layouts/_project_menu.html.haml @@ -5,9 +5,9 @@ - if @project.repo_exists? - if can? current_user, :download_code, @project %li{class: current_controller?(:tree, :blob, :blame) ? 'active' : ''} - = link_to 'Files', project_tree_path(@project, @project.root_ref) + = link_to 'Files', project_tree_path(@project, @ref || @project.root_ref) %li{class: current_controller?(:commit, :commits, :compare, :repositories, :protected_branches) ? 'active' : ''} - = link_to "Commits", project_commits_path(@project, @project.root_ref) + = link_to "Commits", project_commits_path(@project, @ref || @project.root_ref) %li{class: tab_class(:network)} = link_to "Network", graph_project_path(@project) From 275db3c522200b68673c4ea6a16046b3582cda2f Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Tue, 25 Sep 2012 23:39:50 -0400 Subject: [PATCH 47/65] Fix paths in commits/_head --- app/views/commits/_head.html.haml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/views/commits/_head.html.haml b/app/views/commits/_head.html.haml index 86a65f87..edaf8b86 100644 --- a/app/views/commits/_head.html.haml +++ b/app/views/commits/_head.html.haml @@ -1,7 +1,7 @@ %ul.nav.nav-tabs %li= render partial: 'shared/ref_switcher', locals: {destination: 'commits'} - %li{class: "#{'active' if current_page?(project_commits_path(@project)) }"} - = link_to project_commits_path(@project) do + %li{class: "#{'active' if current_controller?(:commit, :commits)}"} + = link_to project_commits_path(@project, @project.root_ref) do Commits %li{class: "#{'active' if current_controller?(:compare)}"} = link_to project_compare_index_path(@project) do @@ -16,7 +16,7 @@ Tags %span.badge= @project.repo.tag_count - - if current_page?(project_commits_path(@project)) && current_user.private_token + - if current_controller?(:commits) && current_user.private_token %li.right %span.rss-icon = link_to project_commits_path(@project, @ref, {format: :atom, private_token: current_user.private_token}), title: "Feed" do From 1799cf3b49dbe243e21f13d51496be7d60d1a73f Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Tue, 25 Sep 2012 23:40:04 -0400 Subject: [PATCH 48/65] Add CommitsController spec to make sure atom feeds work --- spec/controllers/commits_controller_spec.rb | 22 +++++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 spec/controllers/commits_controller_spec.rb diff --git a/spec/controllers/commits_controller_spec.rb b/spec/controllers/commits_controller_spec.rb new file mode 100644 index 00000000..bf335634 --- /dev/null +++ b/spec/controllers/commits_controller_spec.rb @@ -0,0 +1,22 @@ +require 'spec_helper' + +describe CommitsController do + let(:project) { create(:project) } + let(:user) { create(:user) } + + before do + sign_in(user) + + project.add_access(user, :read, :admin) + end + + describe "GET show" do + context "as atom feed" do + it "should render as atom" do + get :show, project_id: project.code, id: "master.atom" + response.should be_success + response.content_type.should == 'application/atom+xml' + end + end + end +end From e9bd45060e3c5198124b4cdad08a7a26f5e0408a Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Tue, 25 Sep 2012 23:56:27 -0400 Subject: [PATCH 49/65] Fix logs not showing in Tree for the root path --- app/controllers/refs_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/refs_controller.rb b/app/controllers/refs_controller.rb index 6ae816ec..e92f4527 100644 --- a/app/controllers/refs_controller.rb +++ b/app/controllers/refs_controller.rb @@ -55,7 +55,7 @@ class RefsController < ApplicationController @commit = CommitDecorator.decorate(@commit) @tree = Tree.new(@commit.tree, project, @ref, params[:path]) @tree = TreeDecorator.new(@tree) - @hex_path = Digest::SHA1.hexdigest(params[:path] || "/") + @hex_path = Digest::SHA1.hexdigest(params[:path] || "") if params[:path] @history_path = project_tree_path(@project, File.join(@ref, params[:path])) From 9f0e80591ab1f3c4967575574fed37754ef2403b Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Wed, 26 Sep 2012 13:21:38 -0400 Subject: [PATCH 50/65] Fix Repository role spec --- app/roles/repository.rb | 3 +++ spec/roles/repository_spec.rb | 12 ++++++------ 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/app/roles/repository.rb b/app/roles/repository.rb index 7118f156..191a6d70 100644 --- a/app/roles/repository.rb +++ b/app/roles/repository.rb @@ -45,14 +45,17 @@ module Repository File.exists?(hook_file) end + # Returns an Array of branch names def branches repo.branches.collect(&:name).sort end + # Returns an Array of tag names def tags repo.tags.collect(&:name).sort.reverse end + # Returns an Array of branch and tag names def ref_names [branches + tags].flatten end diff --git a/spec/roles/repository_spec.rb b/spec/roles/repository_spec.rb index 0fda57a3..c4e0e9de 100644 --- a/spec/roles/repository_spec.rb +++ b/spec/roles/repository_spec.rb @@ -21,27 +21,27 @@ describe Project, "Repository" do end describe "#discover_default_branch" do - let(:master) { double(name: 'master') } - let(:stable) { double(name: 'stable') } + let(:master) { 'master' } + let(:stable) { 'stable' } it "returns 'master' when master exists" do - project.should_receive(:heads).and_return([stable, master]) + project.should_receive(:branches).at_least(:once).and_return([stable, master]) project.discover_default_branch.should == 'master' end it "returns non-master when master exists but default branch is set to something else" do project.default_branch = 'stable' - project.should_receive(:heads).and_return([stable, master]) + project.should_receive(:branches).at_least(:once).and_return([stable, master]) project.discover_default_branch.should == 'stable' end it "returns a non-master branch when only one exists" do - project.should_receive(:heads).and_return([stable]) + project.should_receive(:branches).at_least(:once).and_return([stable]) project.discover_default_branch.should == 'stable' end it "returns nil when no branch exists" do - project.should_receive(:heads).and_return([]) + project.should_receive(:branches).at_least(:once).and_return([]) project.discover_default_branch.should be_nil end end From cf237f1d32c15e1e9fa128fc9e089a7857495f51 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Wed, 26 Sep 2012 13:21:55 -0400 Subject: [PATCH 51/65] Fix GFM request spec --- spec/requests/gitlab_flavored_markdown_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/requests/gitlab_flavored_markdown_spec.rb b/spec/requests/gitlab_flavored_markdown_spec.rb index 2e5d4209..bb00d945 100644 --- a/spec/requests/gitlab_flavored_markdown_spec.rb +++ b/spec/requests/gitlab_flavored_markdown_spec.rb @@ -43,7 +43,7 @@ describe "Gitlab Flavored Markdown" do describe "for commits" do it "should render title in commits#index" do - visit project_commits_path(project, ref: @branch_name) + visit project_commits_path(project, @branch_name) page.should have_link("##{issue.id}") end @@ -69,7 +69,7 @@ describe "Gitlab Flavored Markdown" do end it "should render title in refs#blame" do - visit blame_file_project_ref_path(project, File.join(@branch_name, @test_file)) + visit project_blame_path(project, File.join(@branch_name, @test_file)) within(".blame_commit") do page.should have_link("##{issue.id}") From 67fd7432943657f29d3d417445d54bc7c6d46946 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Wed, 26 Sep 2012 13:22:30 -0400 Subject: [PATCH 52/65] Clean up project access spec --- spec/requests/security/project_access_spec.rb | 170 ++++++++++-------- 1 file changed, 92 insertions(+), 78 deletions(-) diff --git a/spec/requests/security/project_access_spec.rb b/spec/requests/security/project_access_spec.rb index 9b07df6c..5f26b781 100644 --- a/spec/requests/security/project_access_spec.rb +++ b/spec/requests/security/project_access_spec.rb @@ -14,204 +14,218 @@ describe "Application access" do end describe "Project" do + let(:project) { create(:project) } + + let(:master) { create(:user) } + let(:guest) { create(:user) } + let(:reporter) { create(:user) } + before do - @project = Factory :project - @u1 = Factory :user - @u2 = Factory :user - @u3 = Factory :user # full access - @project.users_projects.create(user: @u1, project_access: UsersProject::MASTER) + project.users_projects.create(user: master, project_access: UsersProject::MASTER) + # readonly - @project.users_projects.create(user: @u3, project_access: UsersProject::REPORTER) + project.users_projects.create(user: reporter, project_access: UsersProject::REPORTER) end describe "GET /project_code" do - subject { project_path(@project) } + subject { project_path(project) } - it { should be_allowed_for @u1 } - it { should be_allowed_for @u3 } + it { should be_allowed_for master } + it { should be_allowed_for reporter } it { should be_denied_for :admin } - it { should be_denied_for @u2 } + it { should be_denied_for guest } it { should be_denied_for :user } it { should be_denied_for :visitor } end - describe "GET /project_code/master/tree" do - subject { project_tree_path(@project, @project.root_ref) } + describe "GET /project_code/tree/master" do + subject { project_tree_path(project, project.root_ref) } - it { should be_allowed_for @u1 } - it { should be_allowed_for @u3 } + it { should be_allowed_for master } + it { should be_allowed_for reporter } it { should be_denied_for :admin } - it { should be_denied_for @u2 } + it { should be_denied_for guest } it { should be_denied_for :user } it { should be_denied_for :visitor } end - describe "GET /project_code/commits" do - subject { project_commits_path(@project) } + describe "GET /project_code/commits/master" do + subject { project_commits_path(project, project.root_ref) } - it { should be_allowed_for @u1 } - it { should be_allowed_for @u3 } + it { should be_allowed_for master } + it { should be_allowed_for reporter } it { should be_denied_for :admin } - it { should be_denied_for @u2 } + it { should be_denied_for guest } it { should be_denied_for :user } it { should be_denied_for :visitor } end - describe "GET /project_code/commit" do - subject { project_commit_path(@project, @project.commit.id) } + describe "GET /project_code/commit/:sha" do + subject { project_commit_path(project, project.commit) } - it { should be_allowed_for @u1 } - it { should be_allowed_for @u3 } + it { should be_allowed_for master } + it { should be_allowed_for reporter } it { should be_denied_for :admin } - it { should be_denied_for @u2 } + it { should be_denied_for guest } + it { should be_denied_for :user } + it { should be_denied_for :visitor } + end + + describe "GET /project_code/compare" do + subject { project_compare_index_path(project) } + + it { should be_allowed_for master } + it { should be_allowed_for reporter } + it { should be_denied_for :admin } + it { should be_denied_for guest } it { should be_denied_for :user } it { should be_denied_for :visitor } end describe "GET /project_code/team" do - subject { project_team_index_path(@project) } + subject { project_team_index_path(project) } - it { should be_allowed_for @u1 } - it { should be_allowed_for @u3 } + it { should be_allowed_for master } + it { should be_allowed_for reporter } it { should be_denied_for :admin } - it { should be_denied_for @u2 } + it { should be_denied_for guest } it { should be_denied_for :user } it { should be_denied_for :visitor } end describe "GET /project_code/wall" do - subject { wall_project_path(@project) } + subject { wall_project_path(project) } - it { should be_allowed_for @u1 } - it { should be_allowed_for @u3 } + it { should be_allowed_for master } + it { should be_allowed_for reporter } it { should be_denied_for :admin } - it { should be_denied_for @u2 } + it { should be_denied_for guest } it { should be_denied_for :user } it { should be_denied_for :visitor } end describe "GET /project_code/blob" do before do - commit = @project.commit + commit = project.commit path = commit.tree.contents.select { |i| i.is_a?(Grit::Blob)}.first.name - @blob_path = project_blob_path(@project, File.join(commit.id, path)) + @blob_path = project_blob_path(project, File.join(commit.id, path)) end - it { @blob_path.should be_allowed_for @u1 } - it { @blob_path.should be_allowed_for @u3 } + it { @blob_path.should be_allowed_for master } + it { @blob_path.should be_allowed_for reporter } it { @blob_path.should be_denied_for :admin } - it { @blob_path.should be_denied_for @u2 } + it { @blob_path.should be_denied_for guest } it { @blob_path.should be_denied_for :user } it { @blob_path.should be_denied_for :visitor } end describe "GET /project_code/edit" do - subject { edit_project_path(@project) } + subject { edit_project_path(project) } - it { should be_allowed_for @u1 } - it { should be_denied_for @u3 } + it { should be_allowed_for master } + it { should be_denied_for reporter } it { should be_denied_for :admin } - it { should be_denied_for @u2 } + it { should be_denied_for guest } it { should be_denied_for :user } it { should be_denied_for :visitor } end describe "GET /project_code/deploy_keys" do - subject { project_deploy_keys_path(@project) } + subject { project_deploy_keys_path(project) } - it { should be_allowed_for @u1 } - it { should be_denied_for @u3 } + it { should be_allowed_for master } + it { should be_denied_for reporter } it { should be_denied_for :admin } - it { should be_denied_for @u2 } + it { should be_denied_for guest } it { should be_denied_for :user } it { should be_denied_for :visitor } end describe "GET /project_code/issues" do - subject { project_issues_path(@project) } + subject { project_issues_path(project) } - it { should be_allowed_for @u1 } - it { should be_allowed_for @u3 } + it { should be_allowed_for master } + it { should be_allowed_for reporter } it { should be_denied_for :admin } - it { should be_denied_for @u2 } + it { should be_denied_for guest } it { should be_denied_for :user } it { should be_denied_for :visitor } end describe "GET /project_code/snippets" do - subject { project_snippets_path(@project) } + subject { project_snippets_path(project) } - it { should be_allowed_for @u1 } - it { should be_allowed_for @u3 } + it { should be_allowed_for master } + it { should be_allowed_for reporter } it { should be_denied_for :admin } - it { should be_denied_for @u2 } + it { should be_denied_for guest } it { should be_denied_for :user } it { should be_denied_for :visitor } end describe "GET /project_code/merge_requests" do - subject { project_merge_requests_path(@project) } + subject { project_merge_requests_path(project) } - it { should be_allowed_for @u1 } - it { should be_allowed_for @u3 } + it { should be_allowed_for master } + it { should be_allowed_for reporter } it { should be_denied_for :admin } - it { should be_denied_for @u2 } + it { should be_denied_for guest } it { should be_denied_for :user } it { should be_denied_for :visitor } end describe "GET /project_code/repository" do - subject { project_repository_path(@project) } + subject { project_repository_path(project) } - it { should be_allowed_for @u1 } - it { should be_allowed_for @u3 } + it { should be_allowed_for master } + it { should be_allowed_for reporter } it { should be_denied_for :admin } - it { should be_denied_for @u2 } + it { should be_denied_for guest } it { should be_denied_for :user } it { should be_denied_for :visitor } end describe "GET /project_code/repository/branches" do - subject { branches_project_repository_path(@project) } + subject { branches_project_repository_path(project) } - it { should be_allowed_for @u1 } - it { should be_allowed_for @u3 } + it { should be_allowed_for master } + it { should be_allowed_for reporter } it { should be_denied_for :admin } - it { should be_denied_for @u2 } + it { should be_denied_for guest } it { should be_denied_for :user } it { should be_denied_for :visitor } end describe "GET /project_code/repository/tags" do - subject { tags_project_repository_path(@project) } + subject { tags_project_repository_path(project) } - it { should be_allowed_for @u1 } - it { should be_allowed_for @u3 } + it { should be_allowed_for master } + it { should be_allowed_for reporter } it { should be_denied_for :admin } - it { should be_denied_for @u2 } + it { should be_denied_for guest } it { should be_denied_for :user } it { should be_denied_for :visitor } end describe "GET /project_code/hooks" do - subject { project_hooks_path(@project) } + subject { project_hooks_path(project) } - it { should be_allowed_for @u1 } - it { should be_allowed_for @u3 } + it { should be_allowed_for master } + it { should be_allowed_for reporter } it { should be_denied_for :admin } - it { should be_denied_for @u2 } + it { should be_denied_for guest } it { should be_denied_for :user } it { should be_denied_for :visitor } end describe "GET /project_code/files" do - subject { files_project_path(@project) } + subject { files_project_path(project) } - it { should be_allowed_for @u1 } - it { should be_allowed_for @u3 } + it { should be_allowed_for master } + it { should be_allowed_for reporter } it { should be_denied_for :admin } - it { should be_denied_for @u2 } + it { should be_denied_for guest } it { should be_denied_for :user } it { should be_denied_for :visitor } end From 5cea3e576dfc2a0d01a44b647bbd1c14a202d9e3 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Wed, 26 Sep 2012 13:23:19 -0400 Subject: [PATCH 53/65] Remove atom Dashboard spec that no longer applies --- spec/requests/atom/dashboard_spec.rb | 7 ------- 1 file changed, 7 deletions(-) diff --git a/spec/requests/atom/dashboard_spec.rb b/spec/requests/atom/dashboard_spec.rb index 9459dd01..c160d24a 100644 --- a/spec/requests/atom/dashboard_spec.rb +++ b/spec/requests/atom/dashboard_spec.rb @@ -10,12 +10,5 @@ describe "Dashboard Feed" do page.body.should have_selector("feed title") end end - - context "projects page via private token" do - it "should redirect to login page" do - visit dashboard_path(private_token: user.private_token) - current_path.should == new_user_session_path - end - end end end From 2df3b310f9727956c3ce84322928f3360616f5ad Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Wed, 26 Sep 2012 13:34:51 -0400 Subject: [PATCH 54/65] Rename branches and tags Repo methods to branch_names and tag_names --- app/roles/repository.rb | 14 +++++++------- spec/roles/repository_spec.rb | 8 ++++---- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/app/roles/repository.rb b/app/roles/repository.rb index 191a6d70..e7e57b0e 100644 --- a/app/roles/repository.rb +++ b/app/roles/repository.rb @@ -46,18 +46,18 @@ module Repository end # Returns an Array of branch names - def branches + def branch_names repo.branches.collect(&:name).sort end # Returns an Array of tag names - def tags + def tag_names repo.tags.collect(&:name).sort.reverse end # Returns an Array of branch and tag names def ref_names - [branches + tags].flatten + [branch_names + tag_names].flatten end def repo @@ -112,12 +112,12 @@ module Repository # - If two or more branches are present, returns the one that has a name # matching root_ref (default_branch or 'master' if default_branch is nil) def discover_default_branch - if branches.length == 0 + if branch_names.length == 0 nil - elsif branches.length == 1 - branches.first + elsif branch_names.length == 1 + branch_names.first else - branches.select { |v| v == root_ref }.first + branch_names.select { |v| v == root_ref }.first end end diff --git a/spec/roles/repository_spec.rb b/spec/roles/repository_spec.rb index c4e0e9de..3507585a 100644 --- a/spec/roles/repository_spec.rb +++ b/spec/roles/repository_spec.rb @@ -25,23 +25,23 @@ describe Project, "Repository" do let(:stable) { 'stable' } it "returns 'master' when master exists" do - project.should_receive(:branches).at_least(:once).and_return([stable, master]) + project.should_receive(:branch_names).at_least(:once).and_return([stable, master]) project.discover_default_branch.should == 'master' end it "returns non-master when master exists but default branch is set to something else" do project.default_branch = 'stable' - project.should_receive(:branches).at_least(:once).and_return([stable, master]) + project.should_receive(:branch_names).at_least(:once).and_return([stable, master]) project.discover_default_branch.should == 'stable' end it "returns a non-master branch when only one exists" do - project.should_receive(:branches).at_least(:once).and_return([stable]) + project.should_receive(:branch_names).at_least(:once).and_return([stable]) project.discover_default_branch.should == 'stable' end it "returns nil when no branch exists" do - project.should_receive(:branches).at_least(:once).and_return([]) + project.should_receive(:branch_names).at_least(:once).and_return([]) project.discover_default_branch.should be_nil end end From f8c02f6e39d0a1f752583cc1942bb5e4c53df9c1 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Wed, 26 Sep 2012 13:38:14 -0400 Subject: [PATCH 55/65] Add branches and tags Repo methods Simplifies the actions in RepositoriesController --- app/controllers/repositories_controller.rb | 4 ++-- app/roles/repository.rb | 10 ++++++++++ 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/app/controllers/repositories_controller.rb b/app/controllers/repositories_controller.rb index 583edf8e..614582fa 100644 --- a/app/controllers/repositories_controller.rb +++ b/app/controllers/repositories_controller.rb @@ -14,11 +14,11 @@ class RepositoriesController < ApplicationController end def branches - @branches = @project.repo.heads.sort_by(&:name) + @branches = @project.branches end def tags - @tags = @project.repo.tags.sort_by(&:name).reverse + @tags = @project.tags end def archive diff --git a/app/roles/repository.rb b/app/roles/repository.rb index e7e57b0e..1f44481e 100644 --- a/app/roles/repository.rb +++ b/app/roles/repository.rb @@ -50,11 +50,21 @@ module Repository repo.branches.collect(&:name).sort end + # Returns an Array of Branches + def branches + repo.branches.sort_by(&:name) + end + # Returns an Array of tag names def tag_names repo.tags.collect(&:name).sort.reverse end + # Returns an Array of Tags + def tags + repo.tags.sort_by(&:name).reverse + end + # Returns an Array of branch and tag names def ref_names [branch_names + tag_names].flatten From 7df25e77ac3cfd279a1b1937e45446574d932a3d Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Wed, 26 Sep 2012 13:43:42 -0400 Subject: [PATCH 56/65] Speed up request specs a bit --- spec/requests/gitlab_flavored_markdown_spec.rb | 4 +--- spec/requests/security/project_access_spec.rb | 12 +++++++++++- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/spec/requests/gitlab_flavored_markdown_spec.rb b/spec/requests/gitlab_flavored_markdown_spec.rb index bb00d945..106f6451 100644 --- a/spec/requests/gitlab_flavored_markdown_spec.rb +++ b/spec/requests/gitlab_flavored_markdown_spec.rb @@ -40,10 +40,9 @@ describe "Gitlab Flavored Markdown" do project.add_access(@user, :read, :write) end - describe "for commits" do it "should render title in commits#index" do - visit project_commits_path(project, @branch_name) + visit project_commits_path(project, @branch_name, limit: 1) page.should have_link("##{issue.id}") end @@ -89,7 +88,6 @@ describe "Gitlab Flavored Markdown" do end end - describe "for issues" do before do @other_issue = Factory :issue, diff --git a/spec/requests/security/project_access_spec.rb b/spec/requests/security/project_access_spec.rb index 5f26b781..060a276b 100644 --- a/spec/requests/security/project_access_spec.rb +++ b/spec/requests/security/project_access_spec.rb @@ -51,7 +51,7 @@ describe "Application access" do end describe "GET /project_code/commits/master" do - subject { project_commits_path(project, project.root_ref) } + subject { project_commits_path(project, project.root_ref, limit: 1) } it { should be_allowed_for master } it { should be_allowed_for reporter } @@ -189,6 +189,11 @@ describe "Application access" do describe "GET /project_code/repository/branches" do subject { branches_project_repository_path(project) } + before do + # Speed increase + Project.any_instance.stub(:branches).and_return([]) + end + it { should be_allowed_for master } it { should be_allowed_for reporter } it { should be_denied_for :admin } @@ -200,6 +205,11 @@ describe "Application access" do describe "GET /project_code/repository/tags" do subject { tags_project_repository_path(project) } + before do + # Speed increase + Project.any_instance.stub(:tags).and_return([]) + end + it { should be_allowed_for master } it { should be_allowed_for reporter } it { should be_denied_for :admin } From 82c3f6260366147803d5f63d575231f77c7e73ce Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Wed, 26 Sep 2012 13:58:50 -0400 Subject: [PATCH 57/65] Speed up Compare feature step --- features/project/commits/commits.feature | 6 +++--- features/steps/project/project_browse_commits.rb | 12 +++++------- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/features/project/commits/commits.feature b/features/project/commits/commits.feature index 53de6e6a..df795ef7 100644 --- a/features/project/commits/commits.feature +++ b/features/project/commits/commits.feature @@ -1,8 +1,8 @@ Feature: Project Browse commits Background: Given I sign in as a user - And I own project "Shop" - Given I visit project commits page + And I own a project + And I visit my project's commits page Scenario: I browse commits list for master branch Then I see project commits @@ -18,4 +18,4 @@ Feature: Project Browse commits Scenario: I compare refs Given I visit compare refs page And I fill compare fields with refs - And I see compared refs + Then I see compared refs diff --git a/features/steps/project/project_browse_commits.rb b/features/steps/project/project_browse_commits.rb index 01479987..cb5cabe9 100644 --- a/features/steps/project/project_browse_commits.rb +++ b/features/steps/project/project_browse_commits.rb @@ -4,8 +4,6 @@ class ProjectBrowseCommits < Spinach::FeatureSteps include SharedPaths Then 'I see project commits' do - current_path.should == project_commits_path(@project) - commit = @project.commit page.should have_content(@project.name) page.should have_content(commit.message) @@ -34,14 +32,14 @@ class ProjectBrowseCommits < Spinach::FeatureSteps end And 'I fill compare fields with refs' do - fill_in "from", :with => "master" - fill_in "to", :with => "stable" + fill_in "from", with: "bcf03b5de6c33f3869ef70d68cf06e679d1d7f9a" + fill_in "to", with: "8716fc78f3c65bbf7bcf7b574febd583bc5d2812" click_button "Compare" end - And 'I see compared refs' do - page.should have_content "Commits (27)" + Then 'I see compared refs' do page.should have_content "Compare View" - page.should have_content "Showing 73 changed files" + page.should have_content "Commits (1)" + page.should have_content "Showing 2 changed files" end end From afc4a75499b6678a643e6b62f703f8e7e1eb0f0a Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Wed, 26 Sep 2012 14:52:01 -0400 Subject: [PATCH 58/65] Use Rails.root.join where appropriate --- app/controllers/application_controller.rb | 2 +- app/models/merge_request.rb | 2 +- app/roles/repository.rb | 2 +- config/initializers/1_settings.rb | 2 +- db/fixtures/test/001_repo.rb | 4 ++-- lib/gitlab/backend/gitolite_config.rb | 4 ++-- lib/gitlab/logger.rb | 2 +- lib/gitlab/merge.rb | 4 ++-- lib/gitlab/satellite.rb | 6 +++--- spec/models/project_spec.rb | 2 +- spec/support/stubbed_repository.rb | 4 ++-- 11 files changed, 17 insertions(+), 17 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index f5d978a7..334135c8 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -105,7 +105,7 @@ class ApplicationController < ActionController::Base end def render_404 - render file: File.join(Rails.root, "public", "404"), layout: false, status: "404" + render file: Rails.root.join("public", "404"), layout: false, status: "404" end def require_non_empty_project diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index bb7b53fa..717fe296 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -1,4 +1,4 @@ -require File.join(Rails.root, "app/models/commit") +require Rails.root.join("app/models/commit") class MergeRequest < ActiveRecord::Base include IssueCommonality diff --git a/app/roles/repository.rb b/app/roles/repository.rb index 1f44481e..35093a2f 100644 --- a/app/roles/repository.rb +++ b/app/roles/repository.rb @@ -155,7 +155,7 @@ module Repository # Build file path file_name = self.code + "-" + commit.id.to_s + ".tar.gz" - storage_path = File.join(Rails.root, "tmp", "repositories", self.code) + storage_path = Rails.root.join("tmp", "repositories", self.code) file_path = File.join(storage_path, file_name) # Put files into a directory before archiving diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index 7a7ca43f..276707a7 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -112,7 +112,7 @@ class Settings < Settingslogic def backup_path t = app['backup_path'] || "backups/" - t = /^\//.match(t) ? t : File.join(Rails.root + t) + t = /^\//.match(t) ? t : Rails.root .join(t) t end diff --git a/db/fixtures/test/001_repo.rb b/db/fixtures/test/001_repo.rb index 67d4e7bf..18fc37cd 100644 --- a/db/fixtures/test/001_repo.rb +++ b/db/fixtures/test/001_repo.rb @@ -3,13 +3,13 @@ require 'fileutils' print "Unpacking seed repository..." SEED_REPO = 'seed_project.tar.gz' -REPO_PATH = File.join(Rails.root, 'tmp', 'repositories') +REPO_PATH = Rails.root.join('tmp', 'repositories') # Make whatever directories we need to make FileUtils.mkdir_p(REPO_PATH) # Copy the archive to the repo path -FileUtils.cp(File.join(Rails.root, 'spec', SEED_REPO), REPO_PATH) +FileUtils.cp(Rails.root.join('spec', SEED_REPO), REPO_PATH) # chdir to the repo path FileUtils.cd(REPO_PATH) do diff --git a/lib/gitlab/backend/gitolite_config.rb b/lib/gitlab/backend/gitolite_config.rb index f51e8efc..ffe15fb1 100644 --- a/lib/gitlab/backend/gitolite_config.rb +++ b/lib/gitlab/backend/gitolite_config.rb @@ -10,7 +10,7 @@ module Gitlab attr_reader :config_tmp_dir, :ga_repo, :conf def config_tmp_dir - @config_tmp_dir ||= File.join(Rails.root, 'tmp',"gitlabhq-gitolite-#{Time.now.to_i}") + @config_tmp_dir ||= Rails.root.join('tmp',"gitlabhq-gitolite-#{Time.now.to_i}") end def ga_repo @@ -19,7 +19,7 @@ module Gitlab def apply Timeout::timeout(30) do - File.open(File.join(Rails.root, 'tmp', "gitlabhq-gitolite.lock"), "w+") do |f| + File.open(Rails.root.join('tmp', "gitlabhq-gitolite.lock"), "w+") do |f| begin # Set exclusive lock # to prevent race condition diff --git a/lib/gitlab/logger.rb b/lib/gitlab/logger.rb index 9405163d..cf9a4c4a 100644 --- a/lib/gitlab/logger.rb +++ b/lib/gitlab/logger.rb @@ -15,7 +15,7 @@ module Gitlab end def self.build - new(File.join(Rails.root, "log", file_name)) + new(Rails.root.join("log", file_name)) end end end diff --git a/lib/gitlab/merge.rb b/lib/gitlab/merge.rb index 18013574..de8e737a 100644 --- a/lib/gitlab/merge.rb +++ b/lib/gitlab/merge.rb @@ -28,7 +28,7 @@ module Gitlab def process Grit::Git.with_timeout(30.seconds) do - lock_file = File.join(Rails.root, "tmp", "merge_repo_#{project.path}.lock") + lock_file = Rails.root.join("tmp", "merge_repo_#{project.path}.lock") File.open(lock_file, "w+") do |f| f.flock(File::LOCK_EX) @@ -36,7 +36,7 @@ module Gitlab unless project.satellite.exists? raise "You should run: rake gitlab:app:enable_automerge" end - + project.satellite.clear Dir.chdir(project.satellite.path) do diff --git a/lib/gitlab/satellite.rb b/lib/gitlab/satellite.rb index 4bcbfe8d..9d8dfb8e 100644 --- a/lib/gitlab/satellite.rb +++ b/lib/gitlab/satellite.rb @@ -1,6 +1,6 @@ module Gitlab class Satellite - + PARKING_BRANCH = "__parking_branch" attr_accessor :project @@ -14,7 +14,7 @@ module Gitlab end def path - File.join(Rails.root, "tmp", "repo_satellites", project.path) + Rails.root.join("tmp", "repo_satellites", project.path) end def exists? @@ -36,6 +36,6 @@ module Gitlab end end end - + end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index c313b58e..bb975a93 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -125,7 +125,7 @@ describe Project do it "should return path to repo" do project = Project.new(path: "somewhere") - project.path_to_repo.should == File.join(Rails.root, "tmp", "repositories", "somewhere") + project.path_to_repo.should == Rails.root.join("tmp", "repositories", "somewhere") end it "returns the full web URL for this repo" do diff --git a/spec/support/stubbed_repository.rb b/spec/support/stubbed_repository.rb index 90491e43..5bf3ea46 100644 --- a/spec/support/stubbed_repository.rb +++ b/spec/support/stubbed_repository.rb @@ -5,11 +5,11 @@ module StubbedRepository if new_record? || path == 'newproject' # There are a couple Project specs and features that expect the Project's # path to be in the returned path, so let's patronize them. - File.join(Rails.root, 'tmp', 'repositories', path) + Rails.root.join('tmp', 'repositories', path) else # For everything else, just give it the path to one of our real seeded # repos. - File.join(Rails.root, 'tmp', 'repositories', 'gitlabhq') + Rails.root.join('tmp', 'repositories', 'gitlabhq') end end From aa0c4b77b60acfc85d99e9eacaff25e34b136529 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Wed, 26 Sep 2012 15:06:07 -0400 Subject: [PATCH 59/65] Add current_action? helper --- app/helpers/application_helper.rb | 15 +++++++++++++++ spec/helpers/application_helper_spec.rb | 19 +++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index f874851a..185e7d84 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -1,4 +1,5 @@ require 'digest/md5' + module ApplicationHelper # Check if a particular controller is the current one @@ -15,6 +16,20 @@ module ApplicationHelper args.any? { |v| v.to_s.downcase == controller.controller_name } end + # Check if a partcular action is the current one + # + # args - One or more action names to check + # + # Examples + # + # # On Projects#new + # current_action?(:new) # => true + # current_action?(:create) # => false + # current_action?(:new, :create) # => true + def current_action?(*args) + args.any? { |v| v.to_s.downcase == action_name } + end + def gravatar_icon(user_email = '', size = 40) if Gitlab.config.disable_gravatar? || user_email.blank? 'no_avatar.png' diff --git a/spec/helpers/application_helper_spec.rb b/spec/helpers/application_helper_spec.rb index fb711dd8..a94d5505 100644 --- a/spec/helpers/application_helper_spec.rb +++ b/spec/helpers/application_helper_spec.rb @@ -20,6 +20,25 @@ describe ApplicationHelper do end end + describe 'current_action?' do + before do + stub!(:action_name).and_return('foo') + end + + it "returns true when action matches argument" do + current_action?(:foo).should be_true + end + + it "returns false when action does not match argument" do + current_action?(:bar).should_not be_true + end + + it "should take any number of arguments" do + current_action?(:baz, :bar).should_not be_true + current_action?(:baz, :bar, :foo).should be_true + end + end + describe "gravatar_icon" do let(:user_email) { 'user@email.com' } From f064c84019f7414cb9cfa9e49fb735dba7f495df Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Wed, 26 Sep 2012 16:13:23 -0400 Subject: [PATCH 60/65] Add nav_link helper to TabHelper --- app/helpers/tab_helper.rb | 68 +++++++++++++++++++++++++++++++++ spec/helpers/tab_helper_spec.rb | 44 +++++++++++++++++++++ 2 files changed, 112 insertions(+) create mode 100644 spec/helpers/tab_helper_spec.rb diff --git a/app/helpers/tab_helper.rb b/app/helpers/tab_helper.rb index c26e3467..abd724b9 100644 --- a/app/helpers/tab_helper.rb +++ b/app/helpers/tab_helper.rb @@ -1,4 +1,72 @@ module TabHelper + # Navigation link helper + # + # Returns an `li` element with an 'active' class if the supplied + # controller(s) and/or action(s) currently active. The contents of the + # element is the value passed to the block. + # + # options - The options hash used to determine if the element is "active" (default: {}) + # :controller - One or more controller names to check (optional). + # :action - One or more action names to check (optional). + # :path - A shorthand path, such as 'dashboard#index', to check (optional). + # :html_options - Extra options to be passed to the list element (optional). + # block - An optional block that will become the contents of the returned + # `li` element. + # + # When both :controller and :action are specified, BOTH must match in order + # to be marked as active. When only one is given, either can match. + # + # Examples + # + # # Assuming we're on TreeController#show + # + # # Controller matches, but action doesn't + # nav_link(controller: [:tree, :refs], action: :edit) { "Hello" } + # # => '
  • Hello
    • ' + # + # # Controller matches + # nav_link(controller: [:tree, :refs]) { "Hello" } + # # => '
  • Hello
    • ' + # + # # Shorthand path + # nav_link(path: 'tree#show') { "Hello" } + # # => '
  • Hello
    • ' + # + # # Supplying custom options for the list element + # nav_link(controller: :tree, html_options: {class: 'home'}) { "Hello" } + # # => '
  • Hello
    • ' + # + # Returns a list item element String + def nav_link(options = {}, &block) + if path = options.delete(:path) + c, a, _ = path.split('#') + else + c = options.delete(:controller) + a = options.delete(:action) + end + + if c && a + # When given both options, make sure BOTH are active + klass = current_controller?(*c) && current_action?(*a) ? 'active' : '' + else + # Otherwise check EITHER option + klass = current_controller?(*c) || current_action?(*a) ? 'active' : '' + end + + # Add our custom class into the html_options, which may or may not exist + # and which may or may not already have a :class key + o = options.delete(:html_options) || {} + o[:class] ||= '' + o[:class] += ' ' + klass + o[:class].strip! + + if block_given? + content_tag(:li, capture(&block), o) + else + content_tag(:li, nil, o) + end + end + def tab_class(tab_key) active = case tab_key diff --git a/spec/helpers/tab_helper_spec.rb b/spec/helpers/tab_helper_spec.rb new file mode 100644 index 00000000..ef8e4cf6 --- /dev/null +++ b/spec/helpers/tab_helper_spec.rb @@ -0,0 +1,44 @@ +require 'spec_helper' + +describe TabHelper do + include ApplicationHelper + + describe 'nav_link' do + before do + controller.stub!(:controller_name).and_return('foo') + stub!(:action_name).and_return('foo') + end + + it "captures block output" do + nav_link { "Testing Blocks" }.should match(/Testing Blocks/) + end + + it "performs checks on the current controller" do + nav_link(controller: :foo).should match(/
  • /) + nav_link(controller: :bar).should_not match(/active/) + nav_link(controller: [:foo, :bar]).should match(/active/) + end + + it "performs checks on the current action" do + nav_link(action: :foo).should match(/
  • /) + nav_link(action: :bar).should_not match(/active/) + nav_link(action: [:foo, :bar]).should match(/active/) + end + + it "performs checks on both controller and action when both are present" do + nav_link(controller: :bar, action: :foo).should_not match(/active/) + nav_link(controller: :foo, action: :bar).should_not match(/active/) + nav_link(controller: :foo, action: :foo).should match(/active/) + end + + it "accepts a path shorthand" do + nav_link(path: 'foo#bar').should_not match(/active/) + nav_link(path: 'foo#foo').should match(/active/) + end + + it "passes extra html options to the list element" do + nav_link(action: :foo, html_options: {class: 'home'}).should match(/
  • /) + nav_link(html_options: {class: 'active'}).should match(/
  • /) + end + end +end From 36f68140d1fcd89ed6bd92ac69cf13c566db63d5 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Wed, 26 Sep 2012 16:14:17 -0400 Subject: [PATCH 61/65] Replace various "active tab" checks with nav_link Also remove now-unused tab_class helper --- app/helpers/tab_helper.rb | 44 +------------------ app/views/blame/_head.html.haml | 5 +-- app/views/commits/_head.html.haml | 16 +++---- app/views/issues/_head.html.haml | 15 +++---- app/views/layouts/_app_menu.html.haml | 10 ++--- app/views/layouts/_project_menu.html.haml | 22 +++++----- app/views/layouts/admin.html.haml | 12 ++--- app/views/layouts/profile.html.haml | 18 +++----- app/views/projects/_project_head.html.haml | 20 ++++----- .../repositories/_branches_head.html.haml | 15 +++---- app/views/tree/_head.html.haml | 5 +-- 11 files changed, 61 insertions(+), 121 deletions(-) diff --git a/app/helpers/tab_helper.rb b/app/helpers/tab_helper.rb index abd724b9..c76c1b6f 100644 --- a/app/helpers/tab_helper.rb +++ b/app/helpers/tab_helper.rb @@ -67,48 +67,6 @@ module TabHelper end end - def tab_class(tab_key) - active = case tab_key - - # Project Area - when :wall; wall_tab? - when :wiki; controller.controller_name == "wikis" - when :network; current_page?(controller: "projects", action: "graph", id: @project) - when :merge_requests; controller.controller_name == "merge_requests" - - # Dashboard Area - when :help; controller.controller_name == "help" - when :search; current_page?(search_path) - when :dash_issues; current_page?(dashboard_issues_path) - when :dash_mr; current_page?(dashboard_merge_requests_path) - when :root; current_page?(dashboard_path) || current_page?(root_path) - - # Profile Area - when :profile; current_page?(controller: "profile", action: :show) - when :history; current_page?(controller: "profile", action: :history) - when :account; current_page?(controller: "profile", action: :account) - when :token; current_page?(controller: "profile", action: :token) - when :design; current_page?(controller: "profile", action: :design) - when :ssh_keys; controller.controller_name == "keys" - - # Admin Area - when :admin_root; controller.controller_name == "dashboard" - when :admin_users; controller.controller_name == 'users' - when :admin_projects; controller.controller_name == "projects" - when :admin_hooks; controller.controller_name == 'hooks' - when :admin_resque; controller.controller_name == 'resque' - when :admin_logs; controller.controller_name == 'logs' - - else - false - end - active ? "active" : nil - end - - def wall_tab? - current_page?(controller: "projects", action: "wall", id: @project) - end - def project_tab_class [:show, :files, :edit, :update].each do |action| return "active" if current_page?(controller: "projects", action: action, id: @project) @@ -121,7 +79,7 @@ module TabHelper def branches_tab_class if current_page?(branches_project_repository_path(@project)) || - controller.controller_name == "protected_branches" || + current_controller?(:protected_branches) || current_page?(project_repository_path(@project)) 'active' end diff --git a/app/views/blame/_head.html.haml b/app/views/blame/_head.html.haml index 2a6052a1..3b027bda 100644 --- a/app/views/blame/_head.html.haml +++ b/app/views/blame/_head.html.haml @@ -1,9 +1,8 @@ %ul.nav.nav-tabs %li = render partial: 'shared/ref_switcher', locals: {destination: 'tree', path: params[:path]} - %li{class: "#{'active' if (controller.controller_name == "refs") }"} - = link_to project_tree_path(@project, @ref) do - Source + = nav_link(controller: :refs) do + = link_to 'Source', project_tree_path(@project, @ref) %li.right .input-prepend.project_clone_holder %button{class: "btn small active", :"data-clone" => @project.ssh_url_to_repo} SSH diff --git a/app/views/commits/_head.html.haml b/app/views/commits/_head.html.haml index edaf8b86..26ae32ed 100644 --- a/app/views/commits/_head.html.haml +++ b/app/views/commits/_head.html.haml @@ -1,17 +1,17 @@ %ul.nav.nav-tabs %li= render partial: 'shared/ref_switcher', locals: {destination: 'commits'} - %li{class: "#{'active' if current_controller?(:commit, :commits)}"} - = link_to project_commits_path(@project, @project.root_ref) do - Commits - %li{class: "#{'active' if current_controller?(:compare)}"} - = link_to project_compare_index_path(@project) do - Compare - %li{class: "#{branches_tab_class}"} + + = nav_link(controller: [:commit, :commits]) do + = link_to 'Commits', project_commits_path(@project, @project.root_ref) + = nav_link(controller: :compare) do + = link_to 'Compare', project_compare_index_path(@project) + + = nav_link(html_options: {class: branches_tab_class}) do = link_to project_repository_path(@project) do Branches %span.badge= @project.repo.branch_count - %li{class: "#{'active' if current_page?(tags_project_repository_path(@project)) }"} + = nav_link(controller: :repositories, action: :tags) do = link_to tags_project_repository_path(@project) do Tags %span.badge= @project.repo.tag_count diff --git a/app/views/issues/_head.html.haml b/app/views/issues/_head.html.haml index 8ebe3e05..4294503c 100644 --- a/app/views/issues/_head.html.haml +++ b/app/views/issues/_head.html.haml @@ -1,13 +1,10 @@ %ul.nav.nav-tabs - %li{class: "#{'active' if current_page?(project_issues_path(@project))}"} - = link_to project_issues_path(@project), class: "tab" do - Browse Issues - %li{class: "#{'active' if current_page?(project_milestones_path(@project))}"} - = link_to project_milestones_path(@project), class: "tab" do - Milestones - %li{class: "#{'active' if current_page?(project_labels_path(@project))}"} - = link_to project_labels_path(@project), class: "tab" do - Labels + = nav_link(controller: :issues) do + = link_to 'Browse Issues', project_issues_path(@project), class: "tab" + = nav_link(controller: :milestones) do + = link_to 'Milestones', project_milestones_path(@project), class: "tab" + = nav_link(controller: :labels) do + = link_to 'Labels', project_labels_path(@project), class: "tab" %li.right %span.rss-icon = link_to project_issues_path(@project, :atom, { private_token: current_user.private_token }) do diff --git a/app/views/layouts/_app_menu.html.haml b/app/views/layouts/_app_menu.html.haml index 02531489..8fac1b39 100644 --- a/app/views/layouts/_app_menu.html.haml +++ b/app/views/layouts/_app_menu.html.haml @@ -1,19 +1,19 @@ %ul.main_menu - %li.home{class: tab_class(:root)} + = nav_link(path: 'dashboard#index', html_options: {class: 'home'}) do = link_to "Home", root_path, title: "Home" - %li{class: tab_class(:dash_issues)} + = nav_link(path: 'dashboard#issues') do = link_to dashboard_issues_path do Issues %span.count= current_user.assigned_issues.opened.count - %li{class: tab_class(:dash_mr)} + = nav_link(path: 'dashboard#merge_requests') do = link_to dashboard_merge_requests_path do Merge Requests %span.count= current_user.cared_merge_requests.count - %li{class: tab_class(:search)} + = nav_link(path: 'search#show') do = link_to "Search", search_path - %li{class: tab_class(:help)} + = nav_link(path: 'help#index') do = link_to "Help", help_path diff --git a/app/views/layouts/_project_menu.html.haml b/app/views/layouts/_project_menu.html.haml index d69f9a0b..72c39619 100644 --- a/app/views/layouts/_project_menu.html.haml +++ b/app/views/layouts/_project_menu.html.haml @@ -1,35 +1,33 @@ %ul.main_menu - %li.home{class: project_tab_class} + = nav_link(html_options: {class: "home #{project_tab_class}"}) do = link_to @project.code, project_path(@project), title: "Project" - if @project.repo_exists? - if can? current_user, :download_code, @project - %li{class: current_controller?(:tree, :blob, :blame) ? 'active' : ''} + = nav_link(controller: %w(tree blob blame)) do = link_to 'Files', project_tree_path(@project, @ref || @project.root_ref) - %li{class: current_controller?(:commit, :commits, :compare, :repositories, :protected_branches) ? 'active' : ''} + = nav_link(controller: %w(commit commits compare repositories protected_branches)) do = link_to "Commits", project_commits_path(@project, @ref || @project.root_ref) - %li{class: tab_class(:network)} + = nav_link(path: 'projects#graph') do = link_to "Network", graph_project_path(@project) - if @project.issues_enabled - %li{class: current_controller?(:issues, :milestones, :labels) ? 'active' : ''} + = nav_link(controller: %w(issues milestones labels)) do = link_to project_issues_filter_path(@project) do Issues %span.count.issue_counter= @project.issues.opened.count - if @project.repo_exists? - if @project.merge_requests_enabled - %li{class: tab_class(:merge_requests)} + = nav_link(controller: :merge_requests) do = link_to project_merge_requests_path(@project) do Merge Requests %span.count.merge_counter= @project.merge_requests.opened.count - if @project.wall_enabled - %li{class: tab_class(:wall)} - = link_to wall_project_path(@project) do - Wall + = nav_link(path: 'projects#wall') do + = link_to 'Wall', wall_project_path(@project) - if @project.wiki_enabled - %li{class: tab_class(:wiki)} - = link_to project_wiki_path(@project, :index) do - Wiki + = nav_link(controller: :wikis) do + = link_to 'Wiki', project_wiki_path(@project, :index) diff --git a/app/views/layouts/admin.html.haml b/app/views/layouts/admin.html.haml index 6af0f641..dbb6939d 100644 --- a/app/views/layouts/admin.html.haml +++ b/app/views/layouts/admin.html.haml @@ -6,17 +6,17 @@ = render "layouts/head_panel", title: "Admin area" .container %ul.main_menu - %li.home{class: tab_class(:admin_root)} + = nav_link(controller: :dashboard, html_options: {class: 'home'}) do = link_to "Stats", admin_root_path - %li{class: tab_class(:admin_projects)} + = nav_link(controller: :projects) do = link_to "Projects", admin_projects_path - %li{class: tab_class(:admin_users)} + = nav_link(controller: :users) do = link_to "Users", admin_users_path - %li{class: tab_class(:admin_logs)} + = nav_link(controller: :logs) do = link_to "Logs", admin_logs_path - %li{class: tab_class(:admin_hooks)} + = nav_link(controller: :hooks) do = link_to "Hooks", admin_hooks_path - %li{class: tab_class(:admin_resque)} + = nav_link(controller: :resque) do = link_to "Resque", admin_resque_path .content= yield diff --git a/app/views/layouts/profile.html.haml b/app/views/layouts/profile.html.haml index 62c8db5b..7a54bb7c 100644 --- a/app/views/layouts/profile.html.haml +++ b/app/views/layouts/profile.html.haml @@ -6,23 +6,17 @@ = render "layouts/head_panel", title: "Profile" .container %ul.main_menu - %li.home{class: tab_class(:profile)} + = nav_link(path: 'profile#show', html_options: {class: 'home'}) do = link_to "Profile", profile_path - - %li{class: tab_class(:account)} + = nav_link(path: 'profile#account') do = link_to "Account", profile_account_path - - %li{class: tab_class(:ssh_keys)} + = nav_link(controller: :keys) do = link_to keys_path do SSH Keys %span.count= current_user.keys.count - - %li{class: tab_class(:design)} + = nav_link(path: 'profile#design') do = link_to "Design", profile_design_path - - %li{class: tab_class(:history)} + = nav_link(path: 'profile#history') do = link_to "History", profile_history_path - - .content - = yield + .content= yield diff --git a/app/views/projects/_project_head.html.haml b/app/views/projects/_project_head.html.haml index 4f38bef8..47a79073 100644 --- a/app/views/projects/_project_head.html.haml +++ b/app/views/projects/_project_head.html.haml @@ -1,29 +1,27 @@ %ul.nav.nav-tabs - %li{ class: "#{'active' if current_page?(project_path(@project)) }" } + = nav_link(path: 'projects#show') do = link_to project_path(@project), class: "activities-tab tab" do %i.icon-home Show - %li{ class: " #{'active' if (controller.controller_name == "team_members") || current_page?(project_team_index_path(@project)) }" } + = nav_link(controller: :team_members) do = link_to project_team_index_path(@project), class: "team-tab tab" do %i.icon-user Team - %li{ class: "#{'active' if current_page?(files_project_path(@project)) }" } - = link_to files_project_path(@project), class: "files-tab tab " do - Attachments - %li{ class: " #{'active' if (controller.controller_name == "snippets") }" } - = link_to project_snippets_path(@project), class: "snippets-tab tab" do - Snippets + = nav_link(path: 'projects#files') do + = link_to 'Attachments', files_project_path(@project), class: "files-tab tab" + = nav_link(controller: :snippets) do + = link_to 'Snippets', project_snippets_path(@project), class: "snippets-tab tab" - if can? current_user, :admin_project, @project - %li.right{class: "#{'active' if controller.controller_name == "deploy_keys"}"} + = nav_link(controller: :deploy_keys, html_options: {class: 'right'}) do = link_to project_deploy_keys_path(@project) do %span Deploy Keys - %li.right{class: "#{'active' if controller.controller_name == "hooks" }"} + = nav_link(controller: :hooks, html_options: {class: 'right'}) do = link_to project_hooks_path(@project) do %span Hooks - %li.right{ class: "#{'active' if current_page?(edit_project_path(@project)) }" } + = nav_link(path: 'projects#edit', html_options: {class: 'right'}) do = link_to edit_project_path(@project), class: "stat-tab tab " do %i.icon-edit Edit diff --git a/app/views/repositories/_branches_head.html.haml b/app/views/repositories/_branches_head.html.haml index 6afff627..25a988cf 100644 --- a/app/views/repositories/_branches_head.html.haml +++ b/app/views/repositories/_branches_head.html.haml @@ -1,11 +1,8 @@ = render "commits/head" %ul.nav.nav-pills - %li{class: ("active" if current_page?(project_repository_path(@project)))} - = link_to project_repository_path(@project) do - Recent - %li{class: ("active" if current_page?(project_protected_branches_path(@project)))} - = link_to project_protected_branches_path(@project) do - Protected - %li{class: ("active" if current_page?(branches_project_repository_path(@project)))} - = link_to branches_project_repository_path(@project) do - All + = nav_link(path: 'repositories#show') do + = link_to 'Recent', project_repository_path(@project) + = nav_link(path: 'protected_branches#index') do + = link_to 'Protected', project_protected_branches_path(@project) + = nav_link(path: 'repositories#branches') do + = link_to 'All', branches_project_repository_path(@project) diff --git a/app/views/tree/_head.html.haml b/app/views/tree/_head.html.haml index c7ebbf88..f1b3f63f 100644 --- a/app/views/tree/_head.html.haml +++ b/app/views/tree/_head.html.haml @@ -1,9 +1,8 @@ %ul.nav.nav-tabs %li = render partial: 'shared/ref_switcher', locals: {destination: 'tree', path: @path} - %li{class: "#{'active' if (controller.controller_name == "tree") }"} - = link_to project_tree_path(@project, @ref) do - Source + = nav_link(controller: :tree) do + = link_to 'Source', project_tree_path(@project, @ref) %li.right .input-prepend.project_clone_holder %button{class: "btn small active", :"data-clone" => @project.ssh_url_to_repo} SSH From 8432c9c176d9a5e646fe007a9cd333b31197abeb Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Wed, 26 Sep 2012 16:45:00 -0400 Subject: [PATCH 62/65] Routes are now final --- config/routes.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/config/routes.rb b/config/routes.rb index decf860b..21521a97 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -159,7 +159,6 @@ Gitlab::Application.routes.draw do end end - # XXX: WIP resources :commit, only: [:show], constraints: {id: /[[:alnum:]]{6,40}/} resources :commits, only: [:show], constraints: {id: /.+/} resources :compare, only: [:index, :create] From d8f7748dea5d43a16babd15dd02dcbc76220da5a Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Wed, 26 Sep 2012 17:00:52 -0400 Subject: [PATCH 63/65] Remove app_menu and project_menu partials Now it's consistent across layouts where their main menus are. --- app/views/layouts/_app_menu.html.haml | 19 ------------ app/views/layouts/_project_menu.html.haml | 33 --------------------- app/views/layouts/application.html.haml | 20 +++++++++++-- app/views/layouts/project.html.haml | 36 +++++++++++++++++++++-- 4 files changed, 50 insertions(+), 58 deletions(-) delete mode 100644 app/views/layouts/_app_menu.html.haml delete mode 100644 app/views/layouts/_project_menu.html.haml diff --git a/app/views/layouts/_app_menu.html.haml b/app/views/layouts/_app_menu.html.haml deleted file mode 100644 index 8fac1b39..00000000 --- a/app/views/layouts/_app_menu.html.haml +++ /dev/null @@ -1,19 +0,0 @@ -%ul.main_menu - = nav_link(path: 'dashboard#index', html_options: {class: 'home'}) do - = link_to "Home", root_path, title: "Home" - - = nav_link(path: 'dashboard#issues') do - = link_to dashboard_issues_path do - Issues - %span.count= current_user.assigned_issues.opened.count - - = nav_link(path: 'dashboard#merge_requests') do - = link_to dashboard_merge_requests_path do - Merge Requests - %span.count= current_user.cared_merge_requests.count - - = nav_link(path: 'search#show') do - = link_to "Search", search_path - - = nav_link(path: 'help#index') do - = link_to "Help", help_path diff --git a/app/views/layouts/_project_menu.html.haml b/app/views/layouts/_project_menu.html.haml deleted file mode 100644 index 72c39619..00000000 --- a/app/views/layouts/_project_menu.html.haml +++ /dev/null @@ -1,33 +0,0 @@ -%ul.main_menu - = nav_link(html_options: {class: "home #{project_tab_class}"}) do - = link_to @project.code, project_path(@project), title: "Project" - - - if @project.repo_exists? - - if can? current_user, :download_code, @project - = nav_link(controller: %w(tree blob blame)) do - = link_to 'Files', project_tree_path(@project, @ref || @project.root_ref) - = nav_link(controller: %w(commit commits compare repositories protected_branches)) do - = link_to "Commits", project_commits_path(@project, @ref || @project.root_ref) - = nav_link(path: 'projects#graph') do - = link_to "Network", graph_project_path(@project) - - - if @project.issues_enabled - = nav_link(controller: %w(issues milestones labels)) do - = link_to project_issues_filter_path(@project) do - Issues - %span.count.issue_counter= @project.issues.opened.count - - - if @project.repo_exists? - - if @project.merge_requests_enabled - = nav_link(controller: :merge_requests) do - = link_to project_merge_requests_path(@project) do - Merge Requests - %span.count.merge_counter= @project.merge_requests.opened.count - - - if @project.wall_enabled - = nav_link(path: 'projects#wall') do - = link_to 'Wall', wall_project_path(@project) - - - if @project.wiki_enabled - = nav_link(controller: :wikis) do - = link_to 'Wiki', project_wiki_path(@project, :index) diff --git a/app/views/layouts/application.html.haml b/app/views/layouts/application.html.haml index dda10d5d..40f4f88c 100644 --- a/app/views/layouts/application.html.haml +++ b/app/views/layouts/application.html.haml @@ -5,6 +5,20 @@ = render "layouts/flash" = render "layouts/head_panel", title: "Dashboard" .container - = render partial: "layouts/app_menu" - .content - = yield + %ul.main_menu + = nav_link(path: 'dashboard#index', html_options: {class: 'home'}) do + = link_to "Home", root_path, title: "Home" + = nav_link(path: 'dashboard#issues') do + = link_to dashboard_issues_path do + Issues + %span.count= current_user.assigned_issues.opened.count + = nav_link(path: 'dashboard#merge_requests') do + = link_to dashboard_merge_requests_path do + Merge Requests + %span.count= current_user.cared_merge_requests.count + = nav_link(path: 'search#show') do + = link_to "Search", search_path + = nav_link(path: 'help#index') do + = link_to "Help", help_path + + .content= yield diff --git a/app/views/layouts/project.html.haml b/app/views/layouts/project.html.haml index 56a947d2..b1dbe41c 100644 --- a/app/views/layouts/project.html.haml +++ b/app/views/layouts/project.html.haml @@ -5,7 +5,37 @@ = render "layouts/flash" = render "layouts/head_panel", title: @project.name .container - = render partial: "layouts/project_menu" - .content - = yield + %ul.main_menu + = nav_link(html_options: {class: "home #{project_tab_class}"}) do + = link_to @project.code, project_path(@project), title: "Project" + - if @project.repo_exists? + - if can? current_user, :download_code, @project + = nav_link(controller: %w(tree blob blame)) do + = link_to 'Files', project_tree_path(@project, @ref || @project.root_ref) + = nav_link(controller: %w(commit commits compare repositories protected_branches)) do + = link_to "Commits", project_commits_path(@project, @ref || @project.root_ref) + = nav_link(path: 'projects#graph') do + = link_to "Network", graph_project_path(@project) + + - if @project.issues_enabled + = nav_link(controller: %w(issues milestones labels)) do + = link_to project_issues_filter_path(@project) do + Issues + %span.count.issue_counter= @project.issues.opened.count + + - if @project.repo_exists? && @project.merge_requests_enabled + = nav_link(controller: :merge_requests) do + = link_to project_merge_requests_path(@project) do + Merge Requests + %span.count.merge_counter= @project.merge_requests.opened.count + + - if @project.wall_enabled + = nav_link(path: 'projects#wall') do + = link_to 'Wall', wall_project_path(@project) + + - if @project.wiki_enabled + = nav_link(controller: :wikis) do + = link_to 'Wiki', project_wiki_path(@project, :index) + + .content= yield From beb324c1c6518f4ad20039b4122ef6d59ba430c2 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 27 Sep 2012 09:49:00 +0300 Subject: [PATCH 64/65] Fixed updir method. Fixed breadcrumbs for commits --- app/decorators/tree_decorator.rb | 8 ++++---- app/views/commits/show.html.haml | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/app/decorators/tree_decorator.rb b/app/decorators/tree_decorator.rb index 97981785..38236c2c 100644 --- a/app/decorators/tree_decorator.rb +++ b/app/decorators/tree_decorator.rb @@ -6,7 +6,7 @@ class TreeDecorator < ApplicationDecorator part_path = "" parts = path.split("\/") - #parts = parts[0...-1] if is_blob? + #parts = parts[0...-1] if is_blob? yield(h.link_to("..", "#", remote: :true)) if parts.count > max_links @@ -21,7 +21,7 @@ class TreeDecorator < ApplicationDecorator end def up_dir? - !!path + path.present? end def up_dir_path @@ -36,8 +36,8 @@ class TreeDecorator < ApplicationDecorator def mb_size size = (tree.size / 1024) if size < 1024 - "#{size} KB" - else + "#{size} KB" + else "#{size/1024} MB" end end diff --git a/app/views/commits/show.html.haml b/app/views/commits/show.html.haml index 11ffdb6a..a4f16465 100644 --- a/app/views/commits/show.html.haml +++ b/app/views/commits/show.html.haml @@ -1,6 +1,6 @@ = render "head" -- if params[:path] +- if @path %ul.breadcrumb %li %span.arrow @@ -9,7 +9,7 @@ %span.divider \/ %li - %a{href: "#"}= params[:path].split("/").join(" / ") + %a{href: "#"}= @path.split("/").join(" / ") %div{id: dom_id(@project)} #commits_list= render "commits" From 2c8d3c33ff64ac6af5daf125a2f9ef917e55bcfc Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 27 Sep 2012 09:53:42 +0300 Subject: [PATCH 65/65] Fixed ref switcher --- app/helpers/application_helper.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 185e7d84..911b46c9 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -60,8 +60,8 @@ module ApplicationHelper def grouped_options_refs(destination = :tree) options = [ - ["Branch", @project.repo.heads.map(&:name) ], - [ "Tag", @project.tags ] + ["Branch", @project.branch_names ], + [ "Tag", @project.tag_names ] ] # If reference is commit id -