Download as
Browse Code »

Commit 9ada67881970b05d4be49cbe329b4bf50ffe4f77

Authored by Dmitriy Zaporozhets
1 parent aa8d4d9f
Exists in master and in 4 other branches 6-8-stable, 7-0-stable, 7-0-stable-spb, spb-stable

Split commit_id and noteable_id for Note

Showing 11 changed files with 54 additions and 28 deletions   Show diff stats
app/models/merge_request.rb
  Diff comments   View file @9ada678
... ... @@ -204,7 +204,7 @@ class MergeRequest < ActiveRecord::Base
204 204  
205 205 def mr_and_commit_notes
206 206 commit_ids = commits.map(&:id)
207   - 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)
  207 + 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)
208 208 end
209 209  
210 210 # Returns the raw diff for this merge request
... ...
app/models/note.rb
  Diff comments   View file @9ada678
... ... @@ -20,7 +20,7 @@ require 'file_size_validator'
20 20 class Note < ActiveRecord::Base
21 21  
22 22 attr_accessible :note, :noteable, :noteable_id, :noteable_type, :project_id,
23   - :attachment, :line_code
  23 + :attachment, :line_code, :commit_id
24 24  
25 25 attr_accessor :notify
26 26 attr_accessor :notify_author
... ... @@ -35,10 +35,14 @@ class Note &lt; ActiveRecord::Base
35 35 validates :note, :project, presence: true
36 36 validates :attachment, file_size: { maximum: 10.megabytes.to_i }
37 37  
  38 + validates :noteable_id, presence: true, if: ->(n) { n.noteable_type.present? && n.noteable_type != 'Commit' }
  39 + validates :commit_id, presence: true, if: ->(n) { n.noteable_type == 'Commit' }
  40 +
38 41 mount_uploader :attachment, AttachmentUploader
39 42  
40 43 # Scopes
41   - scope :common, ->{ where(noteable_id: nil) }
  44 + scope :for_commits, ->{ where(noteable_type: "Commit") }
  45 + scope :common, ->{ where(noteable_id: nil, commit_id: nil) }
42 46 scope :today, ->{ where("created_at >= :date", date: Date.today) }
43 47 scope :last_week, ->{ where("created_at >= :date", date: (Date.today - 7.days)) }
44 48 scope :since, ->(day) { where("created_at >= :date", date: (day)) }
... ... @@ -66,7 +70,7 @@ class Note &lt; ActiveRecord::Base
66 70 # override to return commits, which are not active record
67 71 def noteable
68 72 if for_commit?
69   - project.commit(noteable_id)
  73 + project.commit(commit_id)
70 74 else
71 75 super
72 76 end
... ...
app/models/project.rb
  Diff comments   View file @9ada678
... ... @@ -203,15 +203,15 @@ class Project &lt; ActiveRecord::Base
203 203 end
204 204  
205 205 def build_commit_note(commit)
206   - notes.new(noteable_id: commit.id, noteable_type: "Commit")
  206 + notes.new(commit_id: commit.id, noteable_type: "Commit")
207 207 end
208 208  
209 209 def commit_notes(commit)
210   - notes.where(noteable_id: commit.id, noteable_type: "Commit", line_code: nil)
  210 + notes.where(commit_id: commit.id, noteable_type: "Commit", line_code: nil)
211 211 end
212 212  
213 213 def commit_line_notes(commit)
214   - notes.where(noteable_id: commit.id, noteable_type: "Commit").where("line_code IS NOT NULL")
  214 + notes.where(commit_id: commit.id, noteable_type: "Commit").where("line_code IS NOT NULL")
215 215 end
216 216  
217 217 def public?
... ...
app/roles/note_event.rb
  Diff comments   View file @9ada678
1 1 module NoteEvent
2 2 def note_commit_id
3   - target.noteable_id
  3 + target.commit_id
4 4 end
5 5  
6 6 def note_short_commit_id
... ... @@ -16,7 +16,11 @@ module NoteEvent
16 16 end
17 17  
18 18 def note_target_id
19   - target.noteable_id
  19 + if note_commit?
  20 + target.commit_id
  21 + else
  22 + target.noteable_id.to_s
  23 + end
20 24 end
21 25  
22 26 def wall_note?
... ...
app/views/notes/_common_form.html.haml
  Diff comments   View file @9ada678
... ... @@ -7,6 +7,7 @@
7 7 %div= msg
8 8  
9 9 = f.hidden_field :noteable_id
  10 + = f.hidden_field :commit_id
10 11 = f.hidden_field :noteable_type
11 12 = f.text_area :note, size: 255, class: 'note-text js-gfm-input'
12 13 #preview-note.preview_note.hide
... ...
app/views/notes/_per_line_form.html.haml
  Diff comments   View file @9ada678
... ... @@ -11,6 +11,7 @@
11 11 %div= msg
12 12  
13 13 = f.hidden_field :noteable_id
  14 + = f.hidden_field :commit_id
14 15 = f.hidden_field :noteable_type
15 16 = f.hidden_field :line_code
16 17 = f.text_area :note, size: 255, class: 'line-note-text js-gfm-input'
... ...
config/database.yml.postgresql
  Diff comments   View file @9ada678
... ... @@ -9,6 +9,7 @@ production:
9 9 username: postgres
10 10 password:
11 11 # host: localhost
  12 + # port: 5432
12 13 # socket: /tmp/postgresql.sock
13 14  
14 15 #
... ...
db/migrate/20121218164840_move_noteable_commit_to_own_field.rb 0 → 100644
  Diff comments   View file @9ada678
... ... @@ -0,0 +1,15 @@
  1 +class MoveNoteableCommitToOwnField < ActiveRecord::Migration
  2 + def up
  3 + add_column :notes, :commit_id, :string, null: true
  4 + add_column :notes, :new_noteable_id, :integer, null: true
  5 + Note.where(noteable_type: 'Commit').update_all('commit_id = noteable_id')
  6 + Note.where("noteable_type != 'Commit'").update_all('new_noteable_id = CAST (noteable_id AS INTEGER)')
  7 + remove_column :notes, :noteable_id
  8 + rename_column :notes, :new_noteable_id, :noteable_id
  9 + end
  10 +
  11 + def down
  12 + remove_column :notes, :commit_id
  13 + remove_column :notes, :new_noteable_id
  14 + end
  15 +end
... ...
db/schema.rb
  Diff comments   View file @9ada678
... ... @@ -11,7 +11,7 @@
11 11 #
12 12 # It's strongly recommended to check this file into your version control system.
13 13  
14   -ActiveRecord::Schema.define(:version => 20121205201726) do
  14 +ActiveRecord::Schema.define(:version => 20121218164840) do
15 15  
16 16 create_table "events", :force => true do |t|
17 17 t.string "target_type"
... ... @@ -69,19 +69,19 @@ ActiveRecord::Schema.define(:version =&gt; 20121205201726) do
69 69 add_index "keys", ["user_id"], :name => "index_keys_on_user_id"
70 70  
71 71 create_table "merge_requests", :force => true do |t|
72   - t.string "target_branch", :null => false
73   - t.string "source_branch", :null => false
74   - t.integer "project_id", :null => false
  72 + t.string "target_branch", :null => false
  73 + t.string "source_branch", :null => false
  74 + t.integer "project_id", :null => false
75 75 t.integer "author_id"
76 76 t.integer "assignee_id"
77 77 t.string "title"
78   - t.boolean "closed", :default => false, :null => false
79   - t.datetime "created_at", :null => false
80   - t.datetime "updated_at", :null => false
81   - t.text "st_commits", :limit => 2147483647
82   - t.text "st_diffs", :limit => 2147483647
83   - t.boolean "merged", :default => false, :null => false
84   - t.integer "state", :default => 1, :null => false
  78 + t.boolean "closed", :default => false, :null => false
  79 + t.datetime "created_at", :null => false
  80 + t.datetime "updated_at", :null => false
  81 + t.text "st_commits"
  82 + t.text "st_diffs"
  83 + t.boolean "merged", :default => false, :null => false
  84 + t.integer "state", :default => 1, :null => false
85 85 t.integer "milestone_id"
86 86 end
87 87  
... ... @@ -124,7 +124,6 @@ ActiveRecord::Schema.define(:version =&gt; 20121205201726) do
124 124  
125 125 create_table "notes", :force => true do |t|
126 126 t.text "note"
127   - t.string "noteable_id"
128 127 t.string "noteable_type"
129 128 t.integer "author_id"
130 129 t.datetime "created_at", :null => false
... ... @@ -132,10 +131,11 @@ ActiveRecord::Schema.define(:version =&gt; 20121205201726) do
132 131 t.integer "project_id"
133 132 t.string "attachment"
134 133 t.string "line_code"
  134 + t.string "commit_id"
  135 + t.integer "noteable_id"
135 136 end
136 137  
137 138 add_index "notes", ["created_at"], :name => "index_notes_on_created_at"
138   - add_index "notes", ["noteable_id"], :name => "index_notes_on_noteable_id"
139 139 add_index "notes", ["noteable_type"], :name => "index_notes_on_noteable_type"
140 140 add_index "notes", ["project_id"], :name => "index_notes_on_project_id"
141 141  
... ...
spec/models/merge_request_spec.rb
  Diff comments   View file @9ada678
... ... @@ -42,7 +42,7 @@ describe MergeRequest do
42 42  
43 43 before do
44 44 merge_request.stub(:commits) { [merge_request.project.commit] }
45   - create(:note, noteable: merge_request.commits.first)
  45 + create(:note, commit_id: merge_request.commits.first.id, noteable_type: 'Commit')
46 46 create(:note, noteable: merge_request)
47 47 end
48 48  
... ...
spec/models/note_spec.rb
  Diff comments   View file @9ada678
... ... @@ -81,18 +81,18 @@ describe Note do
81 81 describe "Commit notes" do
82 82 before do
83 83 @note = create(:note,
84   - noteable_id: commit.id,
  84 + commit_id: commit.id,
85 85 noteable_type: "Commit")
86 86 end
87 87  
88 88 it "should be accessible through #noteable" do
89   - @note.noteable_id.should == commit.id
  89 + @note.commit_id.should == commit.id
90 90 @note.noteable.should be_a(Commit)
91 91 @note.noteable.should == commit
92 92 end
93 93  
94 94 it "should save a valid note" do
95   - @note.noteable_id.should == commit.id
  95 + @note.commit_id.should == commit.id
96 96 @note.noteable == commit
97 97 end
98 98  
... ... @@ -104,13 +104,13 @@ describe Note do
104 104 describe "Pre-line commit notes" do
105 105 before do
106 106 @note = create(:note,
107   - noteable_id: commit.id,
  107 + commit_id: commit.id,
108 108 noteable_type: "Commit",
109 109 line_code: "0_16_1")
110 110 end
111 111  
112 112 it "should save a valid note" do
113   - @note.noteable_id.should == commit.id
  113 + @note.commit_id.should == commit.id
114 114 @note.noteable.id.should == commit.id
115 115 end
116 116  
... ...