diff --git a/app/decorators/commit_decorator.rb b/app/decorators/commit_decorator.rb index cc8fa97587b..074176ae802 100644 --- a/app/decorators/commit_decorator.rb +++ b/app/decorators/commit_decorator.rb @@ -1,6 +1,15 @@ class CommitDecorator < ApplicationDecorator decorates :commit + # Returns a string describing the commit for use in a link title + # + # Example + # + # "Commit: Alex Denisov - Project git clone panel" + def link_title + "Commit: #{author_name} - #{title}" + end + # Returns the commits title. # # Usually, the commit title is the first line of the commit message. diff --git a/app/models/commit.rb b/app/models/commit.rb index 5c6b4d88d96..15afedcb101 100644 --- a/app/models/commit.rb +++ b/app/models/commit.rb @@ -1,6 +1,7 @@ class Commit include ActiveModel::Conversion include Gitlab::Encode + include StaticModel extend ActiveModel::Naming attr_accessor :commit @@ -22,8 +23,7 @@ class Commit :to_patch, to: :commit - - class << self + class << self def find_or_first(repo, commit_id = nil, root_ref) commit = if commit_id repo.commit(commit_id) @@ -85,7 +85,7 @@ class Commit first = project.commit(to.try(:strip)) last = project.commit(from.try(:strip)) - result = { + result = { commits: [], diffs: [], commit: nil @@ -105,10 +105,6 @@ class Commit end end - def persisted? - false - end - def initialize(raw_commit, head = nil) @commit = raw_commit @head = head @@ -155,7 +151,7 @@ class Commit prev_commit.try :id end - def parents_count + def parents_count parents && parents.count || 0 end end diff --git a/app/roles/static_model.rb b/app/roles/static_model.rb new file mode 100644 index 00000000000..d26c8f47501 --- /dev/null +++ b/app/roles/static_model.rb @@ -0,0 +1,35 @@ +# Provides an ActiveRecord-like interface to a model whose data is not persisted to a database. +module StaticModel + extend ActiveSupport::Concern + + module ClassMethods + # Used by ActiveRecord's polymorphic association to set object_id + def primary_key + 'id' + end + + # Used by ActiveRecord's polymorphic association to set object_type + def base_class + self + end + end + + # Used by AR for fetching attributes + # + # Pass it along if we respond to it. + def [](key) + send(key) if respond_to?(key) + end + + def to_param + id + end + + def persisted? + false + end + + def destroyed? + false + end +end diff --git a/lib/gitlab/markdown.rb b/lib/gitlab/markdown.rb index 75fa835d502..9a07133d0b3 100644 --- a/lib/gitlab/markdown.rb +++ b/lib/gitlab/markdown.rb @@ -100,7 +100,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: "Commit: #{commit.author_name} - #{CommitDecorator.new(commit).title}", class: "gfm gfm-commit #{html_options[:class]}")) + 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]}")) end end end diff --git a/spec/helpers/gitlab_flavored_markdown_spec.rb b/spec/helpers/gitlab_flavored_markdown_spec.rb deleted file mode 100644 index 28bd46ecb99..00000000000 --- a/spec/helpers/gitlab_flavored_markdown_spec.rb +++ /dev/null @@ -1,232 +0,0 @@ -require "spec_helper" - -describe GitlabMarkdownHelper do - before do - @project = Factory(:project) - @commit = @project.repo.commits.first.parents.first - @commit = CommitDecorator.decorate(Commit.new(@commit)) - @other_project = Factory :project, path: "OtherPath", code: "OtherCode" - @fake_user = Factory :user, name: "fred" - end - - describe "#gfm" do - it "should return text if @project is not set" do - @project = nil - - gfm("foo").should == "foo" - end - - describe "referencing a commit" do - it "should link using a full id" do - gfm("Reverts changes from #{@commit.id}").should == "Reverts changes from #{link_to @commit.id, project_commit_path(@project, id: @commit.id), title: "Commit: #{@commit.author_name} - #{@commit.title}", class: "gfm gfm-commit "}" - end - - it "should link using a short id" do - gfm("Backported from #{@commit.id[0, 6]}").should == "Backported from #{link_to @commit.id[0, 6], project_commit_path(@project, id: @commit.id), title: "Commit: #{@commit.author_name} - #{@commit.title}", class: "gfm gfm-commit "}" - end - - it "should link with adjecent text" do - gfm("Reverted (see #{@commit.id})").should == "Reverted (see #{link_to @commit.id, project_commit_path(@project, id: @commit.id), title: "Commit: #{@commit.author_name} - #{@commit.title}", class: "gfm gfm-commit "})" - end - - it "should not link with an invalid id" do - gfm("What happened in 12345678?").should == "What happened in 12345678?" - end - end - - describe "referencing a team member" do - it "should link using a simple name" do - user = Factory :user, name: "barry" - @project.users << user - member = @project.users_projects.where(user_id: user).first - - gfm("@#{user.name} you are right").should == "#{link_to "@#{user.name}", project_team_member_path(@project, member), class: "gfm gfm-team_member "} you are right" - end - - it "should link using a name with dots" do - user = Factory :user, name: "alphA.Beta" - @project.users << user - member = @project.users_projects.where(user_id: user).first - - gfm("@#{user.name} you are right").should == "#{link_to "@#{user.name}", project_team_member_path(@project, member), class: "gfm gfm-team_member "} you are right" - end - - it "should link using name with underscores" do - user = Factory :user, name: "ping_pong_king" - @project.users << user - member = @project.users_projects.where(user_id: user).first - - gfm("@#{user.name} you are right").should == "#{link_to "@#{user.name}", project_team_member_path(@project, member), class: "gfm gfm-team_member "} you are right" - end - - it "should link with adjecent text" do - user = Factory.create(:user, name: "ace") - @project.users << user - member = @project.users_projects.where(user_id: user).first - - gfm("Mail the Admin (@#{user.name})").should == "Mail the Admin (#{link_to "@#{user.name}", project_team_member_path(@project, member), class: "gfm gfm-team_member "})" - end - - it "should add styles" do - user = Factory :user, name: "barry" - @project.users << user - gfm("@#{user.name} you are right").should have_selector(".gfm.gfm-team_member") - end - - it "should not link using a bogus name" do - gfm("What hapened to @foo?").should == "What hapened to @foo?" - end - end - - describe "referencing an issue" do - before do - @issue = Factory :issue, assignee: @fake_user, author: @fake_user, project: @project - @invalid_issue = Factory :issue, assignee: @fake_user, author: @fake_user, project: @other_project - end - - it "should link using a correct id" do - gfm("Fixes ##{@issue.id}").should == "Fixes #{link_to "##{@issue.id}", project_issue_path(@project, @issue), title: "Issue: #{@issue.title}", class: "gfm gfm-issue "}" - end - - it "should link with adjecent text" do - gfm("This has already been discussed (see ##{@issue.id})").should == "This has already been discussed (see #{link_to "##{@issue.id}", project_issue_path(@project, @issue), title: "Issue: #{@issue.title}", class: "gfm gfm-issue "})" - end - - it "should add styles" do - gfm("Fixes ##{@issue.id}").should have_selector(".gfm.gfm-issue") - end - - it "should not link using an invalid id" do - gfm("##{@invalid_issue.id} has been marked duplicate of this").should == "##{@invalid_issue.id} has been marked duplicate of this" - end - end - - describe "referencing a merge request" do - before do - @merge_request = Factory :merge_request, assignee: @fake_user, author: @fake_user, project: @project - @invalid_merge_request = Factory :merge_request, assignee: @fake_user, author: @fake_user, project: @other_project - end - - it "should link using a correct id" do - gfm("Fixed in !#{@merge_request.id}").should == "Fixed in #{link_to "!#{@merge_request.id}", project_merge_request_path(@project, @merge_request), title: "Merge Request: #{@merge_request.title}", class: "gfm gfm-merge_request "}" - end - - it "should link with adjecent text" do - gfm("This has been fixed already (see !#{@merge_request.id})").should == "This has been fixed already (see #{link_to "!#{@merge_request.id}", project_merge_request_path(@project, @merge_request), title: "Merge Request: #{@merge_request.title}", class: "gfm gfm-merge_request "})" - end - - it "should add styles" do - gfm("Fixed in !#{@merge_request.id}").should have_selector(".gfm.gfm-merge_request") - end - - it "should not link using an invalid id" do - gfm("!#{@invalid_merge_request.id} violates our coding guidelines") - end - end - - describe "referencing a snippet" do - before do - @snippet = Factory.create(:snippet, - title: "Render asset to string", - author: @fake_user, - project: @project) - end - - it "should link using a correct id" do - gfm("Check out $#{@snippet.id}").should == "Check out #{link_to "$#{@snippet.id}", project_snippet_path(@project, @snippet), title: "Snippet: #{@snippet.title}", class: "gfm gfm-snippet "}" - end - - it "should link with adjecent text" do - gfm("I have created a snippet for that ($#{@snippet.id})").should == "I have created a snippet for that (#{link_to "$#{@snippet.id}", project_snippet_path(@project, @snippet), title: "Snippet: #{@snippet.title}", class: "gfm gfm-snippet "})" - end - - it "should add styles" do - gfm("Check out $#{@snippet.id}").should have_selector(".gfm.gfm-snippet") - end - - it "should not link using an invalid id" do - gfm("Don't use $1234").should == "Don't use $1234" - end - end - - it "should link to multiple things" do - user = Factory :user, name: "barry" - @project.users << user - member = @project.users_projects.where(user_id: user).first - - gfm("Let @#{user.name} fix the *mess* in #{@commit.id}").should == "Let #{link_to "@#{user.name}", project_team_member_path(@project, member), class: "gfm gfm-team_member "} fix the *mess* in #{link_to @commit.id, project_commit_path(@project, id: @commit.id), title: "Commit: #{@commit.author_name} - #{@commit.title}", class: "gfm gfm-commit "}" - end - - it "should not trip over other stuff" do - gfm("_Please_ *stop* 'helping' and all the other b*$#%' you do.").should == "_Please_ *stop* 'helping' and all the other b*$#%' you do." - end - - it "should not touch HTML entities" do - gfm("We'll accept good pull requests.").should == "We'll accept good pull requests." - end - - it "should forward HTML options to links" do - gfm("fixed in #{@commit.id}", class: "foo").should have_selector("a.foo") - end - end - - describe "#link_to_gfm" do - let(:issue1) { Factory :issue, assignee: @fake_user, author: @fake_user, project: @project } - let(:issue2) { Factory :issue, assignee: @fake_user, author: @fake_user, project: @project } - - it "should handle references nested in links with all the text" do - link_to_gfm("This should finally fix ##{issue1.id} and ##{issue2.id} for real", project_commit_path(@project, id: @commit.id)).should == "#{link_to "This should finally fix ", project_commit_path(@project, id: @commit.id)}#{link_to "##{issue1.id}", project_issue_path(@project, issue1), title: "Issue: #{issue1.title}", class: "gfm gfm-issue "}#{link_to " and ", project_commit_path(@project, id: @commit.id)}#{link_to "##{issue2.id}", project_issue_path(@project, issue2), title: "Issue: #{issue2.title}", class: "gfm gfm-issue "}#{link_to " for real", project_commit_path(@project, id: @commit.id)}" - end - - it "should forward HTML options" do - link_to_gfm("This should finally fix ##{issue1.id} for real", project_commit_path(@project, id: @commit.id), class: "foo").should have_selector(".foo") - end - end - - describe "#markdown" do - before do - @issue = Factory :issue, assignee: @fake_user, author: @fake_user, project: @project - @merge_request = Factory :merge_request, assignee: @fake_user, author: @fake_user, project: @project - @note = Factory.create(:note, - note: "Screenshot of the new feature", - project: @project, - noteable_id: @commit.id, - noteable_type: "Commit", - attachment: "screenshot123.jpg") - @snippet = Factory.create(:snippet, - title: "Render asset to string", - author: @fake_user, - project: @project) - - @other_user = Factory :user, name: "bill" - @project.users << @other_user - @member = @project.users_projects.where(user_id: @other_user).first - end - - it "should handle references in paragraphs" do - markdown("\n\nLorem ipsum dolor sit amet, consectetur adipiscing elit. #{@commit.id} Nam pulvinar sapien eget odio adipiscing at faucibus orci vestibulum.\n").should == "

Lorem ipsum dolor sit amet, consectetur adipiscing elit. #{link_to @commit.id, project_commit_path(@project, id: @commit.id), title: "Commit: #{@commit.author_name} - #{@commit.title}", class: "gfm gfm-commit "} Nam pulvinar sapien eget odio adipiscing at faucibus orci vestibulum.

\n" - end - - it "should handle references in headers" do - markdown("\n# Working around ##{@issue.id} for now\n## Apply !#{@merge_request.id}").should == "

Working around #{link_to "##{@issue.id}", project_issue_path(@project, @issue), title: "Issue: #{@issue.title}", class: "gfm gfm-issue "} for now

\n\n

Apply #{link_to "!#{@merge_request.id}", project_merge_request_path(@project, @merge_request), title: "Merge Request: #{@merge_request.title}", class: "gfm gfm-merge_request "}

\n" - end - - it "should handle references in lists" do - markdown("\n* dark: ##{@issue.id}\n* light by @#{@other_user.name}\n").should == "\n" - end - - it "should handle references in " do - markdown("Apply _!#{@merge_request.id}_ ASAP").should == "

Apply #{link_to "!#{@merge_request.id}", project_merge_request_path(@project, @merge_request), title: "Merge Request: #{@merge_request.title}", class: "gfm gfm-merge_request "} ASAP

\n" - end - - it "should leave code blocks untouched" do - markdown("\n some code from $#{@snippet.id}\n here too\n").should == "
some code from $#{@snippet.id}\nhere too\n
\n
\n" - - markdown("\n```\nsome code from $#{@snippet.id}\nhere too\n```\n").should == "
some code from $#{@snippet.id}\nhere too\n
\n
\n" - end - - it "should leave inline code untouched" do - markdown("\nDon't use `$#{@snippet.id}` here.\n").should == "

Don't use $#{@snippet.id} here.

\n" - end - end -end diff --git a/spec/helpers/gitlab_markdown_helper_spec.rb b/spec/helpers/gitlab_markdown_helper_spec.rb new file mode 100644 index 00000000000..00164e0cdd7 --- /dev/null +++ b/spec/helpers/gitlab_markdown_helper_spec.rb @@ -0,0 +1,288 @@ +require "spec_helper" + +describe GitlabMarkdownHelper do + let!(:project) { create(:project) } + + let(:user) { create(:user, name: 'gfm') } + let(:commit) { CommitDecorator.decorate(project.commit) } + let(:issue) { create(:issue, project: project) } + let(:merge_request) { create(:merge_request, project: project) } + let(:snippet) { create(:snippet, project: project) } + let(:member) { project.users_projects.where(user_id: user).first } + + before do + # Helper expects a @project instance variable + @project = project + end + + describe "#gfm" do + it "should return unaltered text if project is nil" do + actual = "Testing references: ##{issue.id}" + + gfm(actual).should_not == actual + + @project = nil + gfm(actual).should == actual + end + + it "should not alter non-references" do + actual = expected = "_Please_ *stop* 'helping' and all the other b*$#%' you do." + gfm(actual).should == expected + end + + it "should not touch HTML entities" do + actual = expected = "We'll accept good pull requests." + gfm(actual).should == expected + end + + it "should forward HTML options to links" do + gfm("Fixed in #{commit.id}", class: "foo").should have_selector("a.gfm.foo") + end + + describe "referencing a commit" do + let(:expected) { project_commit_path(project, commit) } + + it "should link using a full id" do + actual = "Reverts #{commit.id}" + gfm(actual).should match(expected) + end + + it "should link using a short id" do + actual = "Backported from #{commit.short_id(6)}" + gfm(actual).should match(expected) + end + + it "should link with adjacent text" do + actual = "Reverted (see #{commit.id})" + gfm(actual).should match(expected) + end + + it "should keep whitespace intact" do + actual = "Changes #{commit.id} dramatically" + expected = /Changes #{commit.id}<\/a> dramatically/ + gfm(actual).should match(expected) + end + + it "should not link with an invalid id" do + actual = expected = "What happened in #{commit.id.reverse}" + gfm(actual).should == expected + end + + it "should include a title attribute" do + actual = "Reverts #{commit.id}" + gfm(actual).should match(/title="#{commit.link_title}"/) + end + + it "should include standard gfm classes" do + actual = "Reverts #{commit.id}" + gfm(actual).should match(/class="\s?gfm gfm-commit\s?"/) + end + end + + describe "referencing a team member" do + let(:actual) { "@#{user.name} you are right." } + let(:expected) { project_team_member_path(project, member) } + + before do + project.users << user + end + + it "should link using a simple name" do + gfm(actual).should match(expected) + end + + it "should link using a name with dots" do + user.update_attributes(name: "alphA.Beta") + gfm(actual).should match(expected) + end + + it "should link using name with underscores" do + user.update_attributes(name: "ping_pong_king") + gfm(actual).should match(expected) + end + + it "should link with adjacent text" do + actual = "Mail the admin (@gfm)" + gfm(actual).should match(expected) + end + + it "should keep whitespace intact" do + actual = "Yes, @#{user.name} is right." + expected = /Yes, @#{user.name}<\/a> is right/ + gfm(actual).should match(expected) + end + + it "should not link with an invalid id" do + actual = expected = "@#{user.name.reverse} you are right." + gfm(actual).should == expected + end + + it "should include standard gfm classes" do + gfm(actual).should match(/class="\s?gfm gfm-team_member\s?"/) + end + end + + # Shared examples for referencing an object + # + # Expects the following attributes to be available in the example group: + # + # - object - The object itself + # - reference - The object reference string (e.g., #1234, $1234, !1234) + # + # Currently limited to Snippets, Issues and MergeRequests + shared_examples 'referenced object' do + let(:actual) { "Reference to #{reference}" } + let(:expected) { polymorphic_path([project, object]) } + + it "should link using a valid id" do + gfm(actual).should match(expected) + end + + it "should link with adjacent text" do + # Wrap the reference in parenthesis + gfm(actual.gsub(reference, "(#{reference})")).should match(expected) + + # Append some text to the end of the reference + gfm(actual.gsub(reference, "#{reference}, right?")).should match(expected) + end + + it "should keep whitespace intact" do + actual = "Referenced #{reference} already." + expected = /Referenced [^\s]+<\/a> already/ + gfm(actual).should match(expected) + end + + it "should not link with an invalid id" do + # Modify the reference string so it's still parsed, but is invalid + reference.gsub!(/^(.)(\d+)$/, '\1' + ('\2' * 2)) + gfm(actual).should == actual + end + + it "should include a title attribute" do + title = "#{object.class.to_s.titlecase}: #{object.title}" + gfm(actual).should match(/title="#{title}"/) + end + + it "should include standard gfm classes" do + css = object.class.to_s.underscore + gfm(actual).should match(/class="\s?gfm gfm-#{css}\s?"/) + end + end + + describe "referencing an issue" do + let(:object) { issue } + let(:reference) { "##{issue.id}" } + + include_examples 'referenced object' + end + + describe "referencing a merge request" do + let(:object) { merge_request } + let(:reference) { "!#{merge_request.id}" } + + include_examples 'referenced object' + end + + describe "referencing a snippet" do + let(:object) { snippet } + let(:reference) { "$#{snippet.id}" } + + include_examples 'referenced object' + end + + describe "referencing multiple objects" do + let(:actual) { "!#{merge_request.id} -> #{commit.id} -> ##{issue.id}" } + + it "should link to the merge request" do + expected = project_merge_request_path(project, merge_request) + gfm(actual).should match(expected) + end + + it "should link to the commit" do + expected = project_commit_path(project, commit) + gfm(actual).should match(expected) + end + + it "should link to the issue" do + expected = project_issue_path(project, issue) + gfm(actual).should match(expected) + end + end + end + + describe "#link_to_gfm" do + let(:commit_path) { project_commit_path(project, commit) } + let(:issues) { create_list(:issue, 2, project: project) } + + it "should handle references nested in links with all the text" do + actual = link_to_gfm("This should finally fix ##{issues[0].id} and ##{issues[1].id} for real", commit_path) + + # Break the result into groups of links with their content, without + # closing tags + groups = actual.split("") + + # Leading commit link + groups[0].should match(/href="#{commit_path}"/) + groups[0].should match(/This should finally fix $/) + + # First issue link + groups[1].should match(/href="#{project_issue_path(project, issues[0])}"/) + groups[1].should match(/##{issues[0].id}$/) + + # Internal commit link + groups[2].should match(/href="#{commit_path}"/) + groups[2].should match(/ and /) + + # Second issue link + groups[3].should match(/href="#{project_issue_path(project, issues[1])}"/) + groups[3].should match(/##{issues[1].id}$/) + + # Trailing commit link + groups[4].should match(/href="#{commit_path}"/) + groups[4].should match(/ for real$/) + end + + it "should forward HTML options" do + actual = link_to_gfm("Fixed in #{commit.id}", commit_path, class: 'foo') + actual.should have_selector 'a.gfm.gfm-commit.foo' + end + end + + describe "#markdown" do + it "should handle references in paragraphs" do + markdown("\n\nLorem ipsum dolor sit amet, consectetur adipiscing elit. #{commit.id} Nam pulvinar sapien eget odio adipiscing at faucibus orci vestibulum.\n").should == "

Lorem ipsum dolor sit amet, consectetur adipiscing elit. #{link_to commit.id, project_commit_path(project, commit), title: commit.link_title, class: "gfm gfm-commit "} Nam pulvinar sapien eget odio adipiscing at faucibus orci vestibulum.

\n" + end + + it "should handle references in headers" do + actual = "\n# Working around ##{issue.id}\n## Apply !#{merge_request.id}" + + markdown(actual).should match(%r{Working around ##{issue.id}}) + markdown(actual).should match(%r{Apply !#{merge_request.id}}) + end + + it "should handle references in lists" do + project.users << user + + actual = "\n* dark: ##{issue.id}\n* light by @#{member.user_name}" + + markdown(actual).should match(%r{
  • dark: ##{issue.id}
    • }) + markdown(actual).should match(%r{
  • light by @#{member.user_name}
    • }) + end + + it "should handle references in " do + actual = "Apply _!#{merge_request.id}_ ASAP" + + markdown(actual).should match(%r{Apply !#{merge_request.id}}) + end + + it "should leave code blocks untouched" do + markdown("\n some code from $#{snippet.id}\n here too\n").should == "
    some code from $#{snippet.id}\nhere too\n
    \n
    \n" + + markdown("\n```\nsome code from $#{snippet.id}\nhere too\n```\n").should == "
    some code from $#{snippet.id}\nhere too\n
    \n
    \n" + end + + it "should leave inline code untouched" do + markdown("\nDon't use `$#{snippet.id}` here.\n").should == "

    Don't use $#{snippet.id} here.

    \n" + end + end +end