diff options
author | Robert Speicher <rspeicher@gmail.com> | 2016-03-11 17:46:50 -0500 |
---|---|---|
committer | Robert Speicher <rspeicher@gmail.com> | 2016-03-11 17:46:50 -0500 |
commit | 01f6db4f643380d143772958b22d3f318b1db3a7 (patch) | |
tree | 26fd4477e1ffc4149bece7d72687779de94d68a0 | |
parent | 70bf6dc702b6354c3a00d0b81e7d7c10be25ffb8 (diff) | |
download | gitlab-ce-rs-disallow-blank-line-code.tar.gz |
Disallow blank (non-null) values for a Note's `line_code` attributers-disallow-blank-line-code
It's unclear how these blank values got added, but GitLab.com had a few:
```
irb(main):002:0> Note.where("line_code IS NOT NULL AND line_code = ''").count
=> 439
```
We've added a migration to convert any existing records to use a NULL
value when blank, and updated Note to set blank values to nil before
validation.
-rw-r--r-- | app/models/note.rb | 7 | ||||
-rw-r--r-- | db/migrate/20160307221555_disallow_blank_line_code_on_note.rb | 9 | ||||
-rw-r--r-- | spec/models/note_spec.rb | 8 |
3 files changed, 23 insertions, 1 deletions
diff --git a/app/models/note.rb b/app/models/note.rb index 8b0610ff77e..2e084b5c80c 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -44,6 +44,7 @@ class Note < ActiveRecord::Base delegate :name, :email, to: :author, prefix: true before_validation :set_award! + before_validation :clear_blank_line_code! validates :note, :project, presence: true validates :note, uniqueness: { scope: [:author, :noteable_type, :noteable_id] }, if: ->(n) { n.is_award } @@ -63,7 +64,7 @@ class Note < ActiveRecord::Base scope :nonawards, ->{ where(is_award: false) } scope :for_commit_id, ->(commit_id) { where(noteable_type: "Commit", commit_id: commit_id) } scope :inline, ->{ where("line_code IS NOT NULL") } - scope :not_inline, ->{ where(line_code: [nil, '']) } + scope :not_inline, ->{ where(line_code: nil) } scope :system, ->{ where(system: true) } scope :user, ->{ where(system: false) } scope :common, ->{ where(noteable_type: ["", nil]) } @@ -375,6 +376,10 @@ class Note < ActiveRecord::Base private + def clear_blank_line_code! + self.line_code = nil if self.line_code.blank? + end + def awards_supported? (for_issue? || for_merge_request?) && !for_diff_line? end diff --git a/db/migrate/20160307221555_disallow_blank_line_code_on_note.rb b/db/migrate/20160307221555_disallow_blank_line_code_on_note.rb new file mode 100644 index 00000000000..49e787d9a9a --- /dev/null +++ b/db/migrate/20160307221555_disallow_blank_line_code_on_note.rb @@ -0,0 +1,9 @@ +class DisallowBlankLineCodeOnNote < ActiveRecord::Migration + def up + execute("UPDATE notes SET line_code = NULL WHERE line_code = ''") + end + + def down + # noop + end +end diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb index cd620ea5440..b854de1d3d5 100644 --- a/spec/models/note_spec.rb +++ b/spec/models/note_spec.rb @@ -226,4 +226,12 @@ describe Note, models: true do expect(note.is_award?).to be_falsy end end + + describe 'clear_blank_line_code!' do + it 'clears a blank line code before validation' do + note = build(:note, line_code: ' ') + + expect { note.valid? }.to change(note, :line_code).to(nil) + end + end end |