Merge branch 'fix-sent-notification-position' into 'master'

Allow SentNotification#position to be set as string or hash

Fixes https://sentry.gitlap.com/gitlab/staginggitlabcom/issues/8491/

See merge request !5189

2 files changed, 37 insertions, 0 deletions

diff --git a/app/models/sent_notification.rb b/app/models/sent_notification.rb
index 016172c6d7e..f4bcb49b34d 100644
--- a/app/models/sent_notification.rb
+++ b/app/models/sent_notification.rb
@@ -72,6 +72,19 @@ class SentNotification < ActiveRecord::Base
     end
   end
 
+  def position=(new_position)
+    if new_position.is_a?(String)
+      new_position = JSON.parse(new_position) rescue nil
+    end
+
+    if new_position.is_a?(Hash)
+      new_position = new_position.with_indifferent_access
+      new_position = Gitlab::Diff::Position.new(new_position)
+    end
+
+    super(new_position)
+  end
+
   def to_param
     self.reply_key
   end
diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb
index 54719cbb8d8..d3dddfb4817 100644
--- a/spec/services/notification_service_spec.rb
+++ b/spec/services/notification_service_spec.rb
@@ -293,6 +293,30 @@ describe NotificationService, services: true do
         end
       end
     end
+
+    context "merge request diff note" do
+      let(:project) { create(:project) }
+      let(:user) { create(:user) }
+      let(:merge_request) { create(:merge_request, source_project: project, assignee: user) }
+      let(:note) { create(:diff_note_on_merge_request, project: project, noteable: merge_request) }
+
+      before do
+        build_team(note.project)
+        project.team << [merge_request.author, :master]
+        project.team << [merge_request.assignee, :master]
+      end
+
+      describe :new_note do
+        it "records sent notifications" do
+          # Ensure create SentNotification by noteable = merge_request 6 times, not noteable = note
+          expect(SentNotification).to receive(:record_note).with(note, any_args).exactly(4).times.and_call_original
+
+          notification.new_note(note)
+
+          expect(SentNotification.last.position).to eq(note.position)
+        end
+      end
+    end
   end
 
   describe 'Issues' do