From 125c07fa0aa31e6c83c8fcc90df623c2ef85832f Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 19 Aug 2013 22:10:21 +0300 Subject: [PATCH 01/10] Add iid field to issues and MR --- .../20130819182730_add_internal_ids_to_issues_and_mr.rb | 6 ++++++ db/schema.rb | 4 +++- 2 files changed, 9 insertions(+), 1 deletion(-) create mode 100644 db/migrate/20130819182730_add_internal_ids_to_issues_and_mr.rb diff --git a/db/migrate/20130819182730_add_internal_ids_to_issues_and_mr.rb b/db/migrate/20130819182730_add_internal_ids_to_issues_and_mr.rb new file mode 100644 index 00000000000..e55ae38f144 --- /dev/null +++ b/db/migrate/20130819182730_add_internal_ids_to_issues_and_mr.rb @@ -0,0 +1,6 @@ +class AddInternalIdsToIssuesAndMr < ActiveRecord::Migration + def change + add_column :issues, :iid, :integer + add_column :merge_requests, :iid, :integer + end +end diff --git a/db/schema.rb b/db/schema.rb index e0e7d47b92e..c2ba5b65fc3 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended to check this file into your version control system. -ActiveRecord::Schema.define(:version => 20130812143708) do +ActiveRecord::Schema.define(:version => 20130819182730) do create_table "deploy_keys_projects", :force => true do |t| t.integer "deploy_key_id", :null => false @@ -62,6 +62,7 @@ ActiveRecord::Schema.define(:version => 20130812143708) do t.text "description" t.integer "milestone_id" t.string "state" + t.integer "iid" end add_index "issues", ["assignee_id"], :name => "index_issues_on_assignee_id" @@ -98,6 +99,7 @@ ActiveRecord::Schema.define(:version => 20130812143708) do t.string "state" t.string "merge_status" t.integer "target_project_id", :null => false + t.integer "iid" end add_index "merge_requests", ["assignee_id"], :name => "index_merge_requests_on_assignee_id" From 7047a44a26f0f75ab23c58c47d096060c01256ed Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 19 Aug 2013 22:10:56 +0300 Subject: [PATCH 02/10] Sett iid on create. Use iid as value to_param --- app/models/concerns/issuable.rb | 12 ++++++++++++ app/models/merge_request.rb | 4 ++++ 2 files changed, 16 insertions(+) diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb index 91fb323825d..158c226480f 100644 --- a/app/models/concerns/issuable.rb +++ b/app/models/concerns/issuable.rb @@ -16,6 +16,7 @@ module Issuable validates :author, presence: true validates :title, presence: true, length: { within: 0..255 } + validates :iid, presence: true, numericality: true scope :authored, ->(user) { where(author_id: user) } scope :assigned_to, ->(u) { where(assignee_id: u.id)} @@ -24,6 +25,8 @@ module Issuable scope :unassigned, -> { where("assignee_id IS NULL") } scope :of_projects, ->(ids) { where(project_id: ids) } + validate :set_iid, on: :create + delegate :name, :email, to: :author, @@ -44,6 +47,15 @@ module Issuable end end + def set_iid + max_iid = project.send(self.class.name.tableize).maximum(:iid) + self.iid = max_iid.to_i + 1 + end + + def to_param + iid.to_s + end + def today? Date.today == created_at.to_date end diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index b7df2e40a16..d525ad17537 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -250,6 +250,10 @@ class MergeRequest < ActiveRecord::Base (source_project.root_ref? source_branch) || for_fork? end + def project + target_project + end + private def dump_commits(commits) From 2e5481b74663deddeda66ccd74eb7e38649cccb7 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 19 Aug 2013 22:11:18 +0300 Subject: [PATCH 03/10] Create task to create iid for existing issues/mr --- lib/tasks/migrate/migrate_iids.rake | 31 +++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 lib/tasks/migrate/migrate_iids.rake diff --git a/lib/tasks/migrate/migrate_iids.rake b/lib/tasks/migrate/migrate_iids.rake new file mode 100644 index 00000000000..4d2d49dd6d8 --- /dev/null +++ b/lib/tasks/migrate/migrate_iids.rake @@ -0,0 +1,31 @@ +desc "GITLAB | Build internal ids for issues and merge requests" +task migrate_iids: :environment do + puts 'Issues'.yellow + Issue.where(iid: nil).find_each(batch_size: 100) do |issue| + begin + issue.set_iid + if issue.save + print '.' + else + print 'F' + end + rescue + print 'F' + end + end + + puts 'done' + puts 'Merge Requests'.yellow + MergeRequest.where(iid: nil).find_each(batch_size: 100) do |mr| + begin + mr.set_iid + if mr.save + print '.' + else + print 'F' + end + rescue => ex + print 'F' + end + end +end From c1583c6d74e7a7088cf6d22957851261de965f23 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 19 Aug 2013 22:12:04 +0300 Subject: [PATCH 04/10] Find issues and Mr by iid in controller --- app/controllers/projects/issues_controller.rb | 2 +- app/controllers/projects/merge_requests_controller.rb | 2 +- app/controllers/projects_controller.rb | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index a7f515ac851..4b6e22e3607 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/app/controllers/projects/issues_controller.rb @@ -91,7 +91,7 @@ class Projects::IssuesController < Projects::ApplicationController protected def issue - @issue ||= @project.issues.find(params[:id]) + @issue ||= @project.issues.find_by_iid!(params[:id]) end def authorize_modify_issue! diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index d135cf05b9e..235247f3e52 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -132,7 +132,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController end def merge_request - @merge_request ||= @project.merge_requests.find(params[:id]) + @merge_request ||= @project.merge_requests.find_by_iid!(params[:id]) end def authorize_modify_merge_request! diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 9e88d0b4757..9b4fe5a9b5b 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -104,7 +104,7 @@ class ProjectsController < Projects::ApplicationController def autocomplete_sources @suggestions = { emojis: Emoji.names, - issues: @project.issues.select([:id, :title, :description]), + issues: @project.issues.select([:iid, :title, :description]), members: @project.team.members.sort_by(&:username).map { |user| { username: user.username, name: user.name } } } From 608f3286539fbcdb4727cb77623261fa506381ff Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 19 Aug 2013 22:12:40 +0300 Subject: [PATCH 05/10] Show iid as default id for issues, mr --- app/views/projects/issues/_issue.html.haml | 2 +- app/views/projects/issues/show.html.haml | 2 +- app/views/projects/merge_requests/_merge_request.html.haml | 2 +- app/views/projects/merge_requests/show/_mr_title.html.haml | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/views/projects/issues/_issue.html.haml b/app/views/projects/issues/_issue.html.haml index 0f3c4f752eb..b9a2c18efdc 100644 --- a/app/views/projects/issues/_issue.html.haml +++ b/app/views/projects/issues/_issue.html.haml @@ -4,7 +4,7 @@ = check_box_tag dom_id(issue,"selected"), nil, false, 'data-id' => issue.id, class: "selected_issue", disabled: !can?(current_user, :modify_issue, issue) .issue-title - %span.light= "##{issue.id}" + %span.light= "##{issue.iid}" = link_to_gfm truncate(issue.title, length: 100), project_issue_path(issue.project, issue), class: "row_title" .issue-info diff --git a/app/views/projects/issues/show.html.haml b/app/views/projects/issues/show.html.haml index fc6faa9c19d..d36a831432d 100644 --- a/app/views/projects/issues/show.html.haml +++ b/app/views/projects/issues/show.html.haml @@ -1,5 +1,5 @@ %h3.page-title - Issue ##{@issue.id} + Issue ##{@issue.iid} %small created at diff --git a/app/views/projects/merge_requests/_merge_request.html.haml b/app/views/projects/merge_requests/_merge_request.html.haml index 276436a9c8e..933d78bcbfb 100644 --- a/app/views/projects/merge_requests/_merge_request.html.haml +++ b/app/views/projects/merge_requests/_merge_request.html.haml @@ -1,6 +1,6 @@ %li{ class: mr_css_classes(merge_request) } .merge-request-title - %span.light= "##{merge_request.id}" + %span.light= "##{merge_request.iid}" = link_to_gfm truncate(merge_request.title, length: 80), project_merge_request_path(merge_request.target_project, merge_request), class: "row_title" - if merge_request.merged? %small.pull-right diff --git a/app/views/projects/merge_requests/show/_mr_title.html.haml b/app/views/projects/merge_requests/show/_mr_title.html.haml index 7dfc995121f..096a9167645 100644 --- a/app/views/projects/merge_requests/show/_mr_title.html.haml +++ b/app/views/projects/merge_requests/show/_mr_title.html.haml @@ -1,5 +1,5 @@ %h3.page-title - = "Merge Request ##{@merge_request.id}:" + = "Merge Request ##{@merge_request.iid}:"   -if @merge_request.for_fork? %span.label-branch From 20397091f10f4c00db7cfab7b8598b9a6b7fdec1 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 19 Aug 2013 22:12:59 +0300 Subject: [PATCH 06/10] Show iid in event feed and uatocomplete as default id for issues, mr --- app/assets/javascripts/gfm_auto_complete.js.coffee | 2 +- app/models/event.rb | 4 ++++ app/views/events/event/_common.html.haml | 2 +- config/routes.rb | 2 +- 4 files changed, 7 insertions(+), 3 deletions(-) diff --git a/app/assets/javascripts/gfm_auto_complete.js.coffee b/app/assets/javascripts/gfm_auto_complete.js.coffee index e22761e57a4..77091da8f61 100644 --- a/app/assets/javascripts/gfm_auto_complete.js.coffee +++ b/app/assets/javascripts/gfm_auto_complete.js.coffee @@ -44,7 +44,7 @@ GitLab.GfmAutoComplete = tpl: @Issues.template callbacks: before_save: (issues) -> - $.map issues, (i) -> id: i.id, title: sanitize(i.title), search: "#{i.id} #{i.title}" + $.map issues, (i) -> id: i.iid, title: sanitize(i.title), search: "#{i.iid} #{i.title}" input.one "focus", => $.getJSON(@dataSource).done (data) -> diff --git a/app/models/event.rb b/app/models/event.rb index 759e84bb55a..5839a834e78 100644 --- a/app/models/event.rb +++ b/app/models/event.rb @@ -256,6 +256,10 @@ class Event < ActiveRecord::Base target.commit_id end + def target_iid + target.respond_to?(:iid) ? target.iid : target_id + end + def note_short_commit_id note_commit_id[0..8] end diff --git a/app/views/events/event/_common.html.haml b/app/views/events/event/_common.html.haml index 6989f862f47..a9d3adf41df 100644 --- a/app/views/events/event/_common.html.haml +++ b/app/views/events/event/_common.html.haml @@ -2,7 +2,7 @@ %span.author_name= link_to_author event %span.event_label{class: event.action_name}= event_action_name(event) - if event.target - %strong= link_to "##{event.target_id}", [event.project, event.target] + %strong= link_to "##{event.target_iid}", [event.project, event.target] - else %strong= gfm event.target_title at diff --git a/config/routes.rb b/config/routes.rb index ff84bc15270..5cf47f9ac5e 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -281,7 +281,7 @@ Gitlab::Application.routes.draw do end end - resources :issues, except: [:destroy] do + resources :issues, constraints: {id: /\d+/}, except: [:destroy] do collection do post :bulk_update end From 434c034159f584348b56a322dbcda8dc65b812f2 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 19 Aug 2013 22:53:11 +0300 Subject: [PATCH 07/10] Iid improvemets * call set_iid before validation * make rake task pass event if objects are invalid * show iid in comment event at dashboard --- app/helpers/events_helper.rb | 2 +- app/models/concerns/issuable.rb | 2 +- app/models/event.rb | 8 ++++++++ app/views/search/_result.html.haml | 4 ++-- lib/tasks/migrate/migrate_iids.rake | 6 ++++-- 5 files changed, 16 insertions(+), 6 deletions(-) diff --git a/app/helpers/events_helper.rb b/app/helpers/events_helper.rb index b93b1502b34..a35fd1ea808 100644 --- a/app/helpers/events_helper.rb +++ b/app/helpers/events_helper.rb @@ -109,7 +109,7 @@ module EventsHelper else link_to event_note_target_path(event) do content_tag :strong do - "#{event.note_target_type} ##{truncate event.note_target_id}" + "#{event.note_target_type} ##{truncate event.note_target_iid}" end end end diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb index 158c226480f..fb08a5aa750 100644 --- a/app/models/concerns/issuable.rb +++ b/app/models/concerns/issuable.rb @@ -16,6 +16,7 @@ module Issuable validates :author, presence: true validates :title, presence: true, length: { within: 0..255 } + validate :set_iid, on: :create validates :iid, presence: true, numericality: true scope :authored, ->(user) { where(author_id: user) } @@ -25,7 +26,6 @@ module Issuable scope :unassigned, -> { where("assignee_id IS NULL") } scope :of_projects, ->(ids) { where(project_id: ids) } - validate :set_iid, on: :create delegate :name, :email, diff --git a/app/models/event.rb b/app/models/event.rb index 5839a834e78..702891f13dc 100644 --- a/app/models/event.rb +++ b/app/models/event.rb @@ -284,6 +284,14 @@ class Event < ActiveRecord::Base end end + def note_target_iid + if note_target.respond_to?(:iid) + note_target.iid + else + note_target_id + end.to_s + end + def wall_note? target.noteable_type.blank? end diff --git a/app/views/search/_result.html.haml b/app/views/search/_result.html.haml index fac5fdfd0b1..5f7540d1b16 100644 --- a/app/views/search/_result.html.haml +++ b/app/views/search/_result.html.haml @@ -23,7 +23,7 @@ %li merge request: = link_to [merge_request.target_project, merge_request] do - %span ##{merge_request.id} + %span ##{merge_request.iid} %strong.term = truncate merge_request.title, length: 50 - if merge_request.for_fork? @@ -37,7 +37,7 @@ %li issue: = link_to [issue.project, issue] do - %span ##{issue.id} + %span ##{issue.iid} %strong.term = truncate issue.title, length: 50 %span.light (#{issue.project.name_with_namespace}) diff --git a/lib/tasks/migrate/migrate_iids.rake b/lib/tasks/migrate/migrate_iids.rake index 4d2d49dd6d8..aef2b319df9 100644 --- a/lib/tasks/migrate/migrate_iids.rake +++ b/lib/tasks/migrate/migrate_iids.rake @@ -4,7 +4,7 @@ task migrate_iids: :environment do Issue.where(iid: nil).find_each(batch_size: 100) do |issue| begin issue.set_iid - if issue.save + if issue.update_attribute(:iid, mr.iid) print '.' else print 'F' @@ -19,7 +19,7 @@ task migrate_iids: :environment do MergeRequest.where(iid: nil).find_each(batch_size: 100) do |mr| begin mr.set_iid - if mr.save + if mr.update_attribute(:iid, mr.iid) print '.' else print 'F' @@ -28,4 +28,6 @@ task migrate_iids: :environment do print 'F' end end + + puts 'done' end From a73e068c06532480137b8b9861ed88f42578bb12 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Tue, 20 Aug 2013 16:56:05 +0300 Subject: [PATCH 08/10] Fixing tests after adding iid for issues/mr --- app/helpers/issues_helper.rb | 10 ++--- .../merge_requests_controller_spec.rb | 27 ++++--------- spec/helpers/gitlab_markdown_helper_spec.rb | 38 +++++++++---------- spec/helpers/issues_helper_spec.rb | 6 +-- spec/models/concerns/issuable_spec.rb | 2 + 5 files changed, 37 insertions(+), 46 deletions(-) diff --git a/app/helpers/issues_helper.rb b/app/helpers/issues_helper.rb index 4f433f3c60d..5977c9cbae2 100644 --- a/app/helpers/issues_helper.rb +++ b/app/helpers/issues_helper.rb @@ -37,23 +37,23 @@ module IssuesHelper end end - def url_for_issue(issue_id) + def url_for_issue(issue_iid) return "" if @project.nil? if @project.used_default_issues_tracker? - url = project_issue_url project_id: @project, id: issue_id + url = project_issue_url project_id: @project, id: issue_iid else url = Gitlab.config.issues_tracker[@project.issues_tracker]["issues_url"] - url.gsub(':id', issue_id.to_s) + url.gsub(':id', issue_iid.to_s) .gsub(':project_id', @project.id.to_s) .gsub(':issues_tracker_id', @project.issues_tracker_id.to_s) end end - def title_for_issue(issue_id) + def title_for_issue(issue_iid) return "" if @project.nil? - if @project.used_default_issues_tracker? && issue = @project.issues.where(id: issue_id).first + if @project.used_default_issues_tracker? && issue = @project.issues.where(iid: issue_iid).first issue.title else "" diff --git a/spec/controllers/merge_requests_controller_spec.rb b/spec/controllers/merge_requests_controller_spec.rb index d344e3eb14c..3c1a9cb7268 100644 --- a/spec/controllers/merge_requests_controller_spec.rb +++ b/spec/controllers/merge_requests_controller_spec.rb @@ -8,13 +8,13 @@ describe Projects::MergeRequestsController do before do sign_in(user) project.team << [user, :master] - Projects::MergeRequestsController.any_instance.stub(validates_merge_request: true) + Projects::MergeRequestsController.any_instance.stub(validates_merge_request: true, ) end describe "#show" do shared_examples "export merge as" do |format| it "should generally work" do - get :show, project_id: project.code, id: merge_request.id, format: format + get :show, project_id: project.code, id: merge_request.iid, format: format expect(response).to be_success end @@ -22,11 +22,11 @@ describe Projects::MergeRequestsController do it "should generate it" do MergeRequest.any_instance.should_receive(:"to_#{format}") - get :show, project_id: project.code, id: merge_request.id, format: format + get :show, project_id: project.code, id: merge_request.iid, format: format end it "should render it" do - get :show, project_id: project.code, id: merge_request.id, format: format + get :show, project_id: project.code, id: merge_request.iid, format: format expect(response.body).to eq((merge_request.send(:"to_#{format}",user)).to_s) end @@ -34,7 +34,7 @@ describe Projects::MergeRequestsController do it "should not escape Html" do MergeRequest.any_instance.stub(:"to_#{format}").and_return('HTML entities &<>" ') - get :show, project_id: project.code, id: merge_request.id, format: format + get :show, project_id: project.code, id: merge_request.iid, format: format expect(response.body).to_not include('&') expect(response.body).to_not include('>') @@ -48,7 +48,7 @@ describe Projects::MergeRequestsController do let(:format) { :diff } it "should really only be a git diff" do - get :show, project_id: project.code, id: merge_request.id, format: format + get :show, project_id: project.code, id: merge_request.iid, format: format expect(response.body).to start_with("diff --git") end @@ -59,24 +59,13 @@ describe Projects::MergeRequestsController do let(:format) { :patch } it "should really be a git email patch with commit" do - get :show, project_id: project.code, id: merge_request.id, format: format + get :show, project_id: project.code, id: merge_request.iid, format: format expect(response.body[0..100]).to start_with("From #{merge_request.commits.last.id}") end - # TODO: fix or remove - #it "should contain as many patches as there are commits" do - #get :show, project_id: project.code, id: merge_request.id, format: format - - #patch_count = merge_request.commits.count - #merge_request.commits.each_with_index do |commit, patch_num| - #expect(response.body).to match(/^From #{commit.id}/) - #expect(response.body).to match(/^Subject: \[PATCH #{patch_num}\/#{patch_count}\]/) - #end - #end - it "should contain git diffs" do - get :show, project_id: project.code, id: merge_request.id, format: format + get :show, project_id: project.code, id: merge_request.iid, format: format expect(response.body).to match(/^diff --git/) end diff --git a/spec/helpers/gitlab_markdown_helper_spec.rb b/spec/helpers/gitlab_markdown_helper_spec.rb index 2dcc61e9560..a4db8b4ff7e 100644 --- a/spec/helpers/gitlab_markdown_helper_spec.rb +++ b/spec/helpers/gitlab_markdown_helper_spec.rb @@ -20,7 +20,7 @@ describe GitlabMarkdownHelper do describe "#gfm" do it "should return unaltered text if project is nil" do - actual = "Testing references: ##{issue.id}" + actual = "Testing references: ##{issue.iid}" gfm(actual).should_not == actual @@ -175,14 +175,14 @@ describe GitlabMarkdownHelper do describe "referencing an issue" do let(:object) { issue } - let(:reference) { "##{issue.id}" } + let(:reference) { "##{issue.iid}" } include_examples 'referenced object' end describe "referencing a merge request" do let(:object) { merge_request } - let(:reference) { "!#{merge_request.id}" } + let(:reference) { "!#{merge_request.iid}" } include_examples 'referenced object' end @@ -230,7 +230,7 @@ describe GitlabMarkdownHelper do end describe "referencing multiple objects" do - let(:actual) { "!#{merge_request.id} -> #{commit.id} -> ##{issue.id}" } + let(:actual) { "!#{merge_request.iid} -> #{commit.id} -> ##{issue.iid}" } it "should link to the merge request" do expected = project_merge_request_path(project, merge_request) @@ -299,7 +299,7 @@ describe GitlabMarkdownHelper do 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) + actual = link_to_gfm("This should finally fix ##{issues[0].iid} and ##{issues[1].iid} for real", commit_path) # Break the result into groups of links with their content, without # closing tags @@ -311,7 +311,7 @@ describe GitlabMarkdownHelper do # First issue link groups[1].should match(/href="#{project_issue_url(project, issues[0])}"/) - groups[1].should match(/##{issues[0].id}$/) + groups[1].should match(/##{issues[0].iid}$/) # Internal commit link groups[2].should match(/href="#{commit_path}"/) @@ -319,7 +319,7 @@ describe GitlabMarkdownHelper do # Second issue link groups[3].should match(/href="#{project_issue_url(project, issues[1])}"/) - groups[3].should match(/##{issues[1].id}$/) + groups[3].should match(/##{issues[1].iid}$/) # Trailing commit link groups[4].should match(/href="#{commit_path}"/) @@ -332,7 +332,7 @@ describe GitlabMarkdownHelper do end it "escapes HTML passed in as the body" do - actual = "This is a

test

- see ##{issues[0].id}" + actual = "This is a

test

- see ##{issues[0].iid}" link_to_gfm(actual, commit_path).should match('<h1>test</h1>') end end @@ -345,25 +345,25 @@ describe GitlabMarkdownHelper do end it "should handle references in headers" do - actual = "\n# Working around ##{issue.id}\n## Apply !#{merge_request.id}" + actual = "\n# Working around ##{issue.iid}\n## Apply !#{merge_request.iid}" - markdown(actual).should match(%r{Working around ##{issue.id}}) - markdown(actual).should match(%r{Apply !#{merge_request.id}}) + markdown(actual).should match(%r{Working around ##{issue.iid}}) + markdown(actual).should match(%r{Apply !#{merge_request.iid}}) end it "should handle references in lists" do project.team << [user, :master] - actual = "\n* dark: ##{issue.id}\n* light by @#{member.user.username}" + actual = "\n* dark: ##{issue.iid}\n* light by @#{member.user.username}" - markdown(actual).should match(%r{
  • dark: ##{issue.id}
  • }) + markdown(actual).should match(%r{
  • dark: ##{issue.iid}
  • }) markdown(actual).should match(%r{
  • light by @#{member.user.username}
  • }) end it "should handle references in " do - actual = "Apply _!#{merge_request.id}_ ASAP" + actual = "Apply _!#{merge_request.iid}_ ASAP" - markdown(actual).should match(%r{Apply !#{merge_request.id}}) + markdown(actual).should match(%r{Apply !#{merge_request.iid}}) end it "should leave code blocks untouched" do @@ -379,19 +379,19 @@ describe GitlabMarkdownHelper do end it "should leave ref-like autolinks untouched" do - markdown("look at http://example.tld/#!#{merge_request.id}").should == "

    look at http://example.tld/#!#{merge_request.id}

    \n" + markdown("look at http://example.tld/#!#{merge_request.iid}").should == "

    look at http://example.tld/#!#{merge_request.iid}

    \n" end it "should leave ref-like href of 'manual' links untouched" do - markdown("why not [inspect !#{merge_request.id}](http://example.tld/#!#{merge_request.id})").should == "

    why not inspect !#{merge_request.id}

    \n" + markdown("why not [inspect !#{merge_request.iid}](http://example.tld/#!#{merge_request.iid})").should == "

    why not inspect !#{merge_request.iid}

    \n" end it "should leave ref-like src of images untouched" do - markdown("screen shot: ![some image](http://example.tld/#!#{merge_request.id})").should == "

    screen shot: \"some

    \n" + markdown("screen shot: ![some image](http://example.tld/#!#{merge_request.iid})").should == "

    screen shot: \"some

    \n" end it "should generate absolute urls for refs" do - markdown("##{issue.id}").should include(project_issue_url(project, issue)) + markdown("##{issue.iid}").should include(project_issue_url(project, issue)) end it "should generate absolute urls for emoji" do diff --git a/spec/helpers/issues_helper_spec.rb b/spec/helpers/issues_helper_spec.rb index a1f23073582..3595af32431 100644 --- a/spec/helpers/issues_helper_spec.rb +++ b/spec/helpers/issues_helper_spec.rb @@ -8,7 +8,7 @@ describe IssuesHelper do describe :title_for_issue do it "should return issue title if used internal tracker" do @project = project - title_for_issue(issue.id).should eq issue.title + title_for_issue(issue.iid).should eq issue.title end it "should always return empty string if used external tracker" do @@ -61,7 +61,7 @@ describe IssuesHelper do it "should return internal path if used internal tracker" do @project = project - url_for_issue(issue.id).should match(int_expected) + url_for_issue(issue.iid).should match(int_expected) end it "should return path to external tracker" do @@ -73,7 +73,7 @@ describe IssuesHelper do it "should return empty string if project nil" do @project = nil - url_for_issue(issue.id).should eq "" + url_for_issue(issue.iid).should eq "" end end diff --git a/spec/models/concerns/issuable_spec.rb b/spec/models/concerns/issuable_spec.rb index 551e1753be0..852146ebaec 100644 --- a/spec/models/concerns/issuable_spec.rb +++ b/spec/models/concerns/issuable_spec.rb @@ -11,7 +11,9 @@ describe Issue, "Issuable" do end describe "Validation" do + before { subject.stub(set_iid: false) } it { should validate_presence_of(:project) } + it { should validate_presence_of(:iid) } it { should validate_presence_of(:author) } it { should validate_presence_of(:title) } it { should ensure_length_of(:title).is_at_least(0).is_at_most(255) } From 9acaec7a26d7cb140e4d0215c9f3cef4e42c5087 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Tue, 20 Aug 2013 17:31:26 +0300 Subject: [PATCH 09/10] Searching for issue/mr by iid in markdown --- app/models/project.rb | 2 +- lib/gitlab/markdown.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/project.rb b/app/models/project.rb index de77bf1c666..47916610a75 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -201,7 +201,7 @@ class Project < ActiveRecord::Base def issue_exists?(issue_id) if used_default_issues_tracker? - self.issues.where(id: issue_id).first.present? + self.issues.where(iid: issue_id).first.present? else true end diff --git a/lib/gitlab/markdown.rb b/lib/gitlab/markdown.rb index 95bb22cfc27..61c622a448a 100644 --- a/lib/gitlab/markdown.rb +++ b/lib/gitlab/markdown.rb @@ -181,7 +181,7 @@ module Gitlab end def reference_merge_request(identifier) - if merge_request = @project.merge_requests.where(id: identifier).first + if merge_request = @project.merge_requests.where(iid: identifier).first link_to("!#{identifier}", project_merge_request_url(@project, merge_request), html_options.merge(title: "Merge Request: #{merge_request.title}", class: "gfm gfm-merge_request #{html_options[:class]}")) end end From e9f1c39e7e258e58fa79a8646c3ad224119a0ae5 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Tue, 20 Aug 2013 21:41:14 +0300 Subject: [PATCH 10/10] Fix tests --- .../features/gitlab_flavored_markdown_spec.rb | 26 +++++++++---------- spec/models/project_spec.rb | 4 +-- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/spec/features/gitlab_flavored_markdown_spec.rb b/spec/features/gitlab_flavored_markdown_spec.rb index 349d68399fc..2ea569a6208 100644 --- a/spec/features/gitlab_flavored_markdown_spec.rb +++ b/spec/features/gitlab_flavored_markdown_spec.rb @@ -11,7 +11,7 @@ describe "GitLab Flavored Markdown" do end before do - Commit.any_instance.stub(title: "fix ##{issue.id}\n\nask @#{fred.username} for details") + Commit.any_instance.stub(title: "fix ##{issue.iid}\n\nask @#{fred.username} for details") end let(:commit) { project.repository.commit } @@ -25,13 +25,13 @@ describe "GitLab Flavored Markdown" do it "should render title in commits#index" do visit project_commits_path(project, 'master', limit: 1) - page.should have_link("##{issue.id}") + page.should have_link("##{issue.iid}") end it "should render title in commits#show" do visit project_commit_path(project, commit) - page.should have_link("##{issue.id}") + page.should have_link("##{issue.iid}") end it "should render description in commits#show" do @@ -43,7 +43,7 @@ describe "GitLab Flavored Markdown" do it "should render title in repositories#branches" do visit project_branches_path(project) - page.should have_link("##{issue.id}") + page.should have_link("##{issue.iid}") end end @@ -57,20 +57,20 @@ describe "GitLab Flavored Markdown" do author: @user, assignee: @user, project: project, - title: "fix ##{@other_issue.id}", + title: "fix ##{@other_issue.iid}", description: "ask @#{fred.username} for details") end it "should render subject in issues#index" do visit project_issues_path(project) - page.should have_link("##{@other_issue.id}") + page.should have_link("##{@other_issue.iid}") end it "should render subject in issues#show" do visit project_issue_path(project, @issue) - page.should have_link("##{@other_issue.id}") + page.should have_link("##{@other_issue.iid}") end it "should render details in issues#show" do @@ -83,19 +83,19 @@ describe "GitLab Flavored Markdown" do describe "for merge requests" do before do - @merge_request = create(:merge_request, source_project: project, target_project: project, title: "fix ##{issue.id}") + @merge_request = create(:merge_request, source_project: project, target_project: project, title: "fix ##{issue.iid}") end it "should render title in merge_requests#index" do visit project_merge_requests_path(project) - page.should have_link("##{issue.id}") + page.should have_link("##{issue.iid}") end it "should render title in merge_requests#show" do visit project_merge_request_path(project, @merge_request) - page.should have_link("##{issue.id}") + page.should have_link("##{issue.iid}") end end @@ -104,20 +104,20 @@ describe "GitLab Flavored Markdown" do before do @milestone = create(:milestone, project: project, - title: "fix ##{issue.id}", + title: "fix ##{issue.iid}", description: "ask @#{fred.username} for details") end it "should render title in milestones#index" do visit project_milestones_path(project) - page.should have_link("##{issue.id}") + page.should have_link("##{issue.iid}") end it "should render title in milestones#show" do visit project_milestone_path(project, @milestone) - page.should have_link("##{issue.id}") + page.should have_link("##{issue.iid}") end it "should render description in milestones#show" do diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 05a54f760f1..bca2ad74249 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -201,11 +201,11 @@ describe Project do let(:ext_project) { create(:redmine_project) } it "should be true or if used internal tracker and issue exists" do - project.issue_exists?(existed_issue.id).should be_true + project.issue_exists?(existed_issue.iid).should be_true end it "should be false or if used internal tracker and issue not exists" do - project.issue_exists?(not_existed_issue.id).should be_false + project.issue_exists?(not_existed_issue.iid).should be_false end it "should always be true if used other tracker" do