From 94ce3834986d354582127e7b0bc134362baae07a Mon Sep 17 00:00:00 2001
From: cmrd Senya <senya@riseup.net>
Date: Tue, 19 Jul 2016 14:02:46 +0300
Subject: [PATCH] Introduce NotificationSerializer

And remove the note_html property from the model.
---
 app/controllers/notifications_controller.rb   | 11 ++--
 app/models/notification.rb                    |  6 ---
 app/serializers/notification_serializer.rb    | 20 +++++++
 .../notification_serializer_spec.rb           | 53 +++++++++++++++++++
 4 files changed, 80 insertions(+), 10 deletions(-)
 create mode 100644 app/serializers/notification_serializer.rb
 create mode 100644 spec/serializers/notification_serializer_spec.rb

diff --git a/app/controllers/notifications_controller.rb b/app/controllers/notifications_controller.rb
index 984a381e7..7ddb9b8d6 100644
--- a/app/controllers/notifications_controller.rb
+++ b/app/controllers/notifications_controller.rb
@@ -52,13 +52,16 @@ class NotificationsController < ApplicationController
       format.html
       format.xml { render :xml => @notifications.to_xml }
       format.json {
-        @notifications.each do |note|
-          note.note_html = render_to_string(partial: "notification", locals: {note: note, no_aspect_dropdown: true})
-        end
-        render json: @notifications.to_json
+        render json: @notifications, each_serializer: NotificationSerializer
       }
     end
+  end
 
+  def default_serializer_options
+    {
+      context: self,
+      root:    false
+    }
   end
 
   def read_all
diff --git a/app/models/notification.rb b/app/models/notification.rb
index f9e601fb2..11511462c 100644
--- a/app/models/notification.rb
+++ b/app/models/notification.rb
@@ -8,16 +8,10 @@ class Notification < ActiveRecord::Base
   has_many :actors, class_name: "Person", through: :notification_actors, source: :person
   belongs_to :target, polymorphic: true
 
-  attr_accessor :note_html
-
   def self.for(recipient, opts={})
     where(opts.merge!(recipient_id: recipient.id)).order("updated_at DESC")
   end
 
-  def as_json(opts={})
-    super(opts.merge(methods: :note_html))
-  end
-
   def email_the_user(target, actor)
     recipient.mail(mail_job, recipient_id, actor.id, target.id)
   end
diff --git a/app/serializers/notification_serializer.rb b/app/serializers/notification_serializer.rb
new file mode 100644
index 000000000..c873a9e70
--- /dev/null
+++ b/app/serializers/notification_serializer.rb
@@ -0,0 +1,20 @@
+class NotificationSerializer < ActiveModel::Serializer
+  attributes :id,
+             :target_type,
+             :target_id,
+             :recipient_id,
+             :unread,
+             :created_at,
+             :updated_at,
+             :note_html
+
+  def note_html
+    context.render_to_string(partial: "notifications/notification", locals: {note: object, no_aspect_dropdown: true})
+  end
+
+  def initialize(*_)
+    super
+    self.polymorphic = true
+    self.root = false
+  end
+end
diff --git a/spec/serializers/notification_serializer_spec.rb b/spec/serializers/notification_serializer_spec.rb
new file mode 100644
index 000000000..c771ad2b4
--- /dev/null
+++ b/spec/serializers/notification_serializer_spec.rb
@@ -0,0 +1,53 @@
+require "spec_helper"
+
+describe NotificationSerializer do
+  let(:notifications_controller) { NotificationsController.new }
+
+  before do
+    allow(notifications_controller).to receive(:current_user).and_return(notification.recipient)
+    notifications_controller.request = ActionController::TestRequest.new(host: AppConfig.pod_uri)
+  end
+
+  let(:notification) { FactoryGirl.create(:notification) }
+  let(:json_output) {
+    NotificationSerializer.new(notification, context: notifications_controller).to_json
+  }
+
+  subject(:parsed_hash) {
+    JSON.parse json_output
+  }
+
+  it { is_expected.to have_key(notification.type.demodulize.underscore.to_s) }
+
+  context "typed object" do
+    let(:object) {
+      parsed_hash[notification.type.demodulize.underscore]
+    }
+
+    it "have all properties set" do
+      %w(id target_id recipient_id unread target_type).each do |key|
+        expect(object[key]).to eq(notification.send(key))
+      end
+
+      %w(created_at updated_at).each do |key|
+        expect(object[key]).to eq(notification.send(key).strftime("%FT%T.%LZ"))
+      end
+
+      expect(object).to have_key("note_html")
+    end
+
+    context "note_html" do
+      before do
+        # Nokogiri issue, see https://github.com/sparklemotion/nokogiri/issues/800
+        # TODO: remove when the above issue is fixed
+        expect_any_instance_of(ApplicationHelper).to receive(:timeago).and_return("")
+      end
+
+      let(:note_html) { object["note_html"] }
+
+      it "contains valid html" do
+        expect(Nokogiri::HTML(note_html).errors).to eq([])
+      end
+    end
+  end
+end