Return proper suggestion JSON to the FEosw-suggest-diff-line-change

8 files changed, 113 insertions, 14 deletions

diff --git a/app/models/suggestion.rb b/app/models/suggestion.rb
index 31ba02b2029..03389462572 100644
--- a/app/models/suggestion.rb
+++ b/app/models/suggestion.rb
@@ -1,3 +1,37 @@
 class Suggestion < ApplicationRecord
   belongs_to :note
+
+  def from_line
+    position_new_line
+  end
+
+  def to_line
+    position_new_line
+  end
+
+  def appliable?
+    note.active?
+  end
+
+  private
+
+  # TODO: finish it and use in `appliable?`
+  # def outdated?
+  #   changing.split("\n") == current_changing_lines
+  # end
+
+  # def current_changing_lines
+  #   from_index = from_line - 1
+  #   to_index = to_line - 1
+
+  #   new_blob_lines[from_index..to_index]
+  # end
+
+#   def new_blob_lines
+#     @new_blob_lines ||= note.diff_file.new_blob.lines
+#   end
+
+  def position_new_line
+    note.position.new_line
+  end
 end
diff --git a/app/serializers/note_entity.rb b/app/serializers/note_entity.rb
index d17c684e687..1d3b59eb1b7 100644
--- a/app/serializers/note_entity.rb
+++ b/app/serializers/note_entity.rb
@@ -36,10 +36,7 @@ class NoteEntity < API::Entities::Note
     end
   end
 
-  expose :suggestions do |note|
-    []
-  end
-
+  expose :suggestions, using: SuggestionEntity
   expose :resolved?, as: :resolved
   expose :resolvable?, as: :resolvable
 
diff --git a/app/serializers/suggestion_entity.rb b/app/serializers/suggestion_entity.rb
new file mode 100644
index 00000000000..4de9c85ced3
--- /dev/null
+++ b/app/serializers/suggestion_entity.rb
@@ -0,0 +1,10 @@
+# frozen_string_literal: true
+
+class SuggestionEntity < Grape::Entity
+  expose :from_line
+  expose :to_line
+  expose :appliable?, as: :appliable
+  expose :changing
+  expose :suggestion
+  expose :position
+end
diff --git a/app/services/notes/update_service.rb b/app/services/notes/update_service.rb
index 90e87682a8c..d2e2e3ffcf9 100644
--- a/app/services/notes/update_service.rb
+++ b/app/services/notes/update_service.rb
@@ -15,10 +15,9 @@ module Notes
       end
 
       if note.is_a?(DiffNote)
-        Suggestion.transaction do
-          note.suggestions.delete_all
-          Suggestions::CreateService.new(note).execute
-        end
+        # TODO: understand why delete_all does not work here.
+        note.suggestions.destroy_all
+        Suggestions::CreateService.new(note).execute
       end
 
       note
diff --git a/db/migrate/20181123144235_create_suggestions.rb b/db/migrate/20181123144235_create_suggestions.rb
index ec0572907b5..10322a97de8 100644
--- a/db/migrate/20181123144235_create_suggestions.rb
+++ b/db/migrate/20181123144235_create_suggestions.rb
@@ -4,7 +4,10 @@ class CreateSuggestions < ActiveRecord::Migration[5.0]
       t.text :changing, null: false
       t.text :suggestion, null: false
       t.integer :position, null: false
-      t.references :note, foreign_key: { on_delete: :cascade }, index: true
+      t.references :note,
+        foreign_key: { on_delete: :cascade },
+        index: true,
+        null: false
     end
   end
 end
diff --git a/db/schema.rb b/db/schema.rb
index 1434ec21422..229d1b743e6 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -1930,7 +1930,7 @@ ActiveRecord::Schema.define(version: 20181123144235) do
     t.text "changing", null: false
     t.text "suggestion", null: false
     t.integer "position", null: false
-    t.integer "note_id"
+    t.integer "note_id", null: false
     t.index ["note_id"], name: "index_suggestions_on_note_id", using: :btree
   end
 
@@ -2405,7 +2405,7 @@ ActiveRecord::Schema.define(version: 20181123144235) do
   add_foreign_key "services", "projects", name: "fk_71cce407f9", on_delete: :cascade
   add_foreign_key "snippets", "projects", name: "fk_be41fd4bb7", on_delete: :cascade
   add_foreign_key "subscriptions", "projects", on_delete: :cascade
-  add_foreign_key "suggestions", "notes"
+  add_foreign_key "suggestions", "notes", on_delete: :cascade
   add_foreign_key "system_note_metadata", "notes", name: "fk_d83a918cb1", on_delete: :cascade
   add_foreign_key "term_agreements", "application_setting_terms", column: "term_id"
   add_foreign_key "term_agreements", "users", on_delete: :cascade
diff --git a/spec/models/suggestion_spec.rb b/spec/models/suggestion_spec.rb
index af2c4f84516..3c03fc981e0 100644
--- a/spec/models/suggestion_spec.rb
+++ b/spec/models/suggestion_spec.rb
@@ -1,5 +1,17 @@
-require 'rails_helper'
+require 'spec_helper'
 
-RSpec.describe Suggestion, type: :model do
-  pending "add some examples to (or delete) #{__FILE__}"
+describe Suggestion do
+  let(:suggestion) { create(:suggestion, position: 0) }
+
+  describe '#appliable?' do
+    context 'when note not active' do
+      it 'returns false' do
+      end
+    end
+
+    context 'when current changing lines does not match persisted' do
+      it 'returns false' do
+      end
+    end
+  end
 end
diff --git a/spec/serializers/suggestion_entity_spec.rb b/spec/serializers/suggestion_entity_spec.rb
new file mode 100644
index 00000000000..2d23ab26eea
--- /dev/null
+++ b/spec/serializers/suggestion_entity_spec.rb
@@ -0,0 +1,44 @@
+require 'spec_helper'
+
+describe SuggestionEntity do
+  let(:project_with_repo) { create(:project, :repository) }
+  let(:merge_request) do
+    create(:merge_request, source_project: project_with_repo,
+                           target_project: project_with_repo)
+  end
+
+  let(:position) do
+    Gitlab::Diff::Position.new(old_path: "files/ruby/popen.rb",
+                               new_path: "files/ruby/popen.rb",
+                               old_line: nil,
+                               new_line: 14,
+                               diff_refs: merge_request.diff_refs)
+  end
+
+  let(:diff_note) do
+    create(:diff_note_on_merge_request, project: project_with_repo,
+                                        noteable: merge_request,
+                                        position: position)
+  end
+
+  let!(:suggestion) do
+    create(:suggestion, note: diff_note,
+                        changing: '    vars = {',
+                        suggestion: 'bar',
+                        position: 0)
+  end
+
+  let(:entity) do
+    described_class.new(suggestion)
+  end
+
+  context 'as json' do
+    subject { entity.as_json }
+
+    it 'exposes attributes' do
+      expect(subject).to include(:from_line, :to_line,
+                                 :appliable, :changing,
+                                 :suggestion)
+    end
+  end
+end