From 9ada67881970b05d4be49cbe329b4bf50ffe4f77 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Tue, 18 Dec 2012 20:02:00 +0200 Subject: Split commit_id and noteable_id for Note --- app/models/merge_request.rb | 2 +- app/models/note.rb | 10 ++++++--- app/models/project.rb | 6 ++--- app/roles/note_event.rb | 8 +++++-- app/views/notes/_common_form.html.haml | 1 + app/views/notes/_per_line_form.html.haml | 1 + config/database.yml.postgresql | 1 + ...1218164840_move_noteable_commit_to_own_field.rb | 15 +++++++++++++ db/schema.rb | 26 +++++++++++----------- spec/models/merge_request_spec.rb | 2 +- spec/models/note_spec.rb | 10 ++++----- 11 files changed, 54 insertions(+), 28 deletions(-) create mode 100644 db/migrate/20121218164840_move_noteable_commit_to_own_field.rb diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 2409fb80acb..052e0850d96 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -204,7 +204,7 @@ class MergeRequest < ActiveRecord::Base def mr_and_commit_notes commit_ids = commits.map(&:id) - Note.where("(noteable_type = 'MergeRequest' AND noteable_id = :mr_id) OR (noteable_type = 'Commit' AND noteable_id IN (:commit_ids))", mr_id: id, commit_ids: commit_ids) + Note.where("(noteable_type = 'MergeRequest' AND noteable_id = :mr_id) OR (noteable_type = 'Commit' AND commit_id IN (:commit_ids))", mr_id: id, commit_ids: commit_ids) end # Returns the raw diff for this merge request diff --git a/app/models/note.rb b/app/models/note.rb index 219ed9b9332..28b3879239f 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -20,7 +20,7 @@ require 'file_size_validator' class Note < ActiveRecord::Base attr_accessible :note, :noteable, :noteable_id, :noteable_type, :project_id, - :attachment, :line_code + :attachment, :line_code, :commit_id attr_accessor :notify attr_accessor :notify_author @@ -35,10 +35,14 @@ class Note < ActiveRecord::Base validates :note, :project, presence: true validates :attachment, file_size: { maximum: 10.megabytes.to_i } + validates :noteable_id, presence: true, if: ->(n) { n.noteable_type.present? && n.noteable_type != 'Commit' } + validates :commit_id, presence: true, if: ->(n) { n.noteable_type == 'Commit' } + mount_uploader :attachment, AttachmentUploader # Scopes - scope :common, ->{ where(noteable_id: nil) } + scope :for_commits, ->{ where(noteable_type: "Commit") } + scope :common, ->{ where(noteable_id: nil, commit_id: nil) } scope :today, ->{ where("created_at >= :date", date: Date.today) } scope :last_week, ->{ where("created_at >= :date", date: (Date.today - 7.days)) } scope :since, ->(day) { where("created_at >= :date", date: (day)) } @@ -66,7 +70,7 @@ class Note < ActiveRecord::Base # override to return commits, which are not active record def noteable if for_commit? - project.commit(noteable_id) + project.commit(commit_id) else super end diff --git a/app/models/project.rb b/app/models/project.rb index 5871bc67f69..eb6e7cb1b08 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -203,15 +203,15 @@ class Project < ActiveRecord::Base end def build_commit_note(commit) - notes.new(noteable_id: commit.id, noteable_type: "Commit") + notes.new(commit_id: commit.id, noteable_type: "Commit") end def commit_notes(commit) - notes.where(noteable_id: commit.id, noteable_type: "Commit", line_code: nil) + notes.where(commit_id: commit.id, noteable_type: "Commit", line_code: nil) end def commit_line_notes(commit) - notes.where(noteable_id: commit.id, noteable_type: "Commit").where("line_code IS NOT NULL") + notes.where(commit_id: commit.id, noteable_type: "Commit").where("line_code IS NOT NULL") end def public? diff --git a/app/roles/note_event.rb b/app/roles/note_event.rb index 884ab432ec8..db4ced0c095 100644 --- a/app/roles/note_event.rb +++ b/app/roles/note_event.rb @@ -1,6 +1,6 @@ module NoteEvent def note_commit_id - target.noteable_id + target.commit_id end def note_short_commit_id @@ -16,7 +16,11 @@ module NoteEvent end def note_target_id - target.noteable_id + if note_commit? + target.commit_id + else + target.noteable_id.to_s + end end def wall_note? diff --git a/app/views/notes/_common_form.html.haml b/app/views/notes/_common_form.html.haml index 0725082d504..d76be75bc27 100644 --- a/app/views/notes/_common_form.html.haml +++ b/app/views/notes/_common_form.html.haml @@ -7,6 +7,7 @@ %div= msg = f.hidden_field :noteable_id + = f.hidden_field :commit_id = f.hidden_field :noteable_type = f.text_area :note, size: 255, class: 'note-text js-gfm-input' #preview-note.preview_note.hide diff --git a/app/views/notes/_per_line_form.html.haml b/app/views/notes/_per_line_form.html.haml index c8d79850162..ff80ad4e0d5 100644 --- a/app/views/notes/_per_line_form.html.haml +++ b/app/views/notes/_per_line_form.html.haml @@ -11,6 +11,7 @@ %div= msg = f.hidden_field :noteable_id + = f.hidden_field :commit_id = f.hidden_field :noteable_type = f.hidden_field :line_code = f.text_area :note, size: 255, class: 'line-note-text js-gfm-input' diff --git a/config/database.yml.postgresql b/config/database.yml.postgresql index 17b38f3d385..0e873d2b8fb 100644 --- a/config/database.yml.postgresql +++ b/config/database.yml.postgresql @@ -9,6 +9,7 @@ production: username: postgres password: # host: localhost + # port: 5432 # socket: /tmp/postgresql.sock # diff --git a/db/migrate/20121218164840_move_noteable_commit_to_own_field.rb b/db/migrate/20121218164840_move_noteable_commit_to_own_field.rb new file mode 100644 index 00000000000..556ac9e7898 --- /dev/null +++ b/db/migrate/20121218164840_move_noteable_commit_to_own_field.rb @@ -0,0 +1,15 @@ +class MoveNoteableCommitToOwnField < ActiveRecord::Migration + def up + add_column :notes, :commit_id, :string, null: true + add_column :notes, :new_noteable_id, :integer, null: true + Note.where(noteable_type: 'Commit').update_all('commit_id = noteable_id') + Note.where("noteable_type != 'Commit'").update_all('new_noteable_id = CAST (noteable_id AS INTEGER)') + remove_column :notes, :noteable_id + rename_column :notes, :new_noteable_id, :noteable_id + end + + def down + remove_column :notes, :commit_id + remove_column :notes, :new_noteable_id + end +end diff --git a/db/schema.rb b/db/schema.rb index 923d2c5543e..e4a1ec4da97 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 => 20121205201726) do +ActiveRecord::Schema.define(:version => 20121218164840) do create_table "events", :force => true do |t| t.string "target_type" @@ -69,19 +69,19 @@ ActiveRecord::Schema.define(:version => 20121205201726) do add_index "keys", ["user_id"], :name => "index_keys_on_user_id" create_table "merge_requests", :force => true do |t| - t.string "target_branch", :null => false - t.string "source_branch", :null => false - t.integer "project_id", :null => false + t.string "target_branch", :null => false + t.string "source_branch", :null => false + t.integer "project_id", :null => false t.integer "author_id" t.integer "assignee_id" t.string "title" - t.boolean "closed", :default => false, :null => false - t.datetime "created_at", :null => false - t.datetime "updated_at", :null => false - t.text "st_commits", :limit => 2147483647 - t.text "st_diffs", :limit => 2147483647 - t.boolean "merged", :default => false, :null => false - t.integer "state", :default => 1, :null => false + t.boolean "closed", :default => false, :null => false + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false + t.text "st_commits" + t.text "st_diffs" + t.boolean "merged", :default => false, :null => false + t.integer "state", :default => 1, :null => false t.integer "milestone_id" end @@ -124,7 +124,6 @@ ActiveRecord::Schema.define(:version => 20121205201726) do create_table "notes", :force => true do |t| t.text "note" - t.string "noteable_id" t.string "noteable_type" t.integer "author_id" t.datetime "created_at", :null => false @@ -132,10 +131,11 @@ ActiveRecord::Schema.define(:version => 20121205201726) do t.integer "project_id" t.string "attachment" t.string "line_code" + t.string "commit_id" + t.integer "noteable_id" end add_index "notes", ["created_at"], :name => "index_notes_on_created_at" - add_index "notes", ["noteable_id"], :name => "index_notes_on_noteable_id" add_index "notes", ["noteable_type"], :name => "index_notes_on_noteable_type" add_index "notes", ["project_id"], :name => "index_notes_on_project_id" diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index d70647f668d..a0849401254 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -42,7 +42,7 @@ describe MergeRequest do before do merge_request.stub(:commits) { [merge_request.project.commit] } - create(:note, noteable: merge_request.commits.first) + create(:note, commit_id: merge_request.commits.first.id, noteable_type: 'Commit') create(:note, noteable: merge_request) end diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb index 4f9352b9a14..61aaf6455eb 100644 --- a/spec/models/note_spec.rb +++ b/spec/models/note_spec.rb @@ -81,18 +81,18 @@ describe Note do describe "Commit notes" do before do @note = create(:note, - noteable_id: commit.id, + commit_id: commit.id, noteable_type: "Commit") end it "should be accessible through #noteable" do - @note.noteable_id.should == commit.id + @note.commit_id.should == commit.id @note.noteable.should be_a(Commit) @note.noteable.should == commit end it "should save a valid note" do - @note.noteable_id.should == commit.id + @note.commit_id.should == commit.id @note.noteable == commit end @@ -104,13 +104,13 @@ describe Note do describe "Pre-line commit notes" do before do @note = create(:note, - noteable_id: commit.id, + commit_id: commit.id, noteable_type: "Commit", line_code: "0_16_1") end it "should save a valid note" do - @note.noteable_id.should == commit.id + @note.commit_id.should == commit.id @note.noteable.id.should == commit.id end -- cgit v1.2.1