Remove notifiable access from articles and comments notifications (#5415) [deploy]

* Add created_at to comment notification data

* Remove notifiable usage from notifications partials

* Fix typo

app/controllers/notifications_controller.rb

@@ -33,12 +33,6 @@ class NotificationsController < ApplicationController
 
     @notifications = @notifications.limit(num)
 
-    # after notifications are paginated/limited, we check if any of the notifiables is "Comment" with no action
-    # because its respective view is the only one that actually uses ".notifiable" to render,
-    # this way we save useless eager loadings
-    notifiable_eager_loading_params = { notifiable_type: %w[Comment], action: [nil, ""] }
-    @notifications = @notifications.includes(:notifiable) if @notifications.exists?(notifiable_eager_loading_params)
-
     @notifications = NotificationDecorator.decorate_collection(@notifications)
 
     @last_user_reaction = @user.reactions.last&.id

app/services/notifications.rb

@@ -18,6 +18,7 @@ module Notifications
       class: { name: "Comment" },
       path: comment.path,
       processed_html: comment.processed_html,
+      created_at: comment.created_at,
       updated_at: comment.updated_at,
       ancestry: comment.ancestry,
       depth: comment.depth,

app/services/notifications/update.rb

@@ -24,7 +24,9 @@ module Notifications
       new_json_data = notifications.first.json_data || {}
       new_json_data[notifiable.class.name.downcase] = public_send("#{notifiable.class.name.downcase}_data", notifiable)
       new_json_data[:user] = user_data(notifiable.user)
-      new_json_data[:organization] = organization_data(notifiable.organization) if notifiable.is_a?(Article) && notifiable.organization_id
+      add_organization_data = notifiable.is_a?(Article) && notifiable.organization_id
+      new_json_data[:organization] = organization_data(notifiable.organization) if add_organization_data
+
       notifications.update_all(json_data: new_json_data)
     end
 

app/views/notifications/_article.html.erb

@@ -29,11 +29,7 @@
     <% end %>
 
     <div>
-      <% if json_data["article"]["published_at"] %>
-        <small><%= time_ago_in_words json_data["article"]["published_at"] %> ago</small>
-      <% else %>
-        <small><%= time_ago_in_words notification.notifiable.published_at %> ago</small>
-      <% end %>
+      <small><%= time_ago_in_words json_data["article"]["published_at"] %> ago</small>
     </div>
 
     <a href="<%= json_data["article"]["path"] %>">

app/views/notifications/_comment.html.erb

@@ -20,9 +20,11 @@
       <a href="<%= json_data["comment"]["commentable"]["path"] %>">
         <%= h(json_data["comment"]["commentable"]["title"]) %>
       </a>
-      <div>
-        <small><%= time_ago_in_words notification.notifiable.created_at %> ago</small>
-      </div>
+      <% if json_data["comment"]["created_at"] %>
+        <div>
+          <small><%= time_ago_in_words json_data["comment"]["created_at"] %> ago</small>
+        </div>
+      <% end %>
       <%= render "notifications/shared/comment_box", json_data: json_data, notification: notification, context: "default" %>
     <% elsif notification.action == "Moderation" %>
       <a href="/<%= json_data["comment"]["path"].split("/")[1] %>">@<%= json_data["comment"]["path"].split("/")[1] %></a> just left a comment. Since they are new to the community, could you leave a nice reply to help them feel welcome?

spec/services/notifications/new_comment/send_spec.rb

@@ -17,6 +17,30 @@ RSpec.describe Notifications::NewComment::Send, type: :service do
     end.to change(Notification, :count).by(2)
   end
 
+  it "creates a correct user notification" do
+    described_class.call(child_comment)
+
+    notification = child_comment.notifications.last
+
+    expect(notification.action).to be_nil
+    expect(notification.json_data["user"]["id"]).to eq(child_comment.user.id)
+    expect(notification.json_data["user"]["username"]).to eq(child_comment.user.username)
+  end
+
+  it "creates the correct comment data for the notification" do
+    described_class.call(child_comment)
+
+    notification = child_comment.notifications.last
+    json_data = notification.json_data
+
+    expect(json_data["comment"]["id"]).to eq(child_comment.id)
+    expect(Time.zone.parse(json_data["comment"]["created_at"]).to_i).to eq(child_comment.created_at.to_i)
+    expect(Time.zone.parse(json_data["comment"]["updated_at"]).to_i).to eq(child_comment.updated_at.to_i)
+    expect(json_data["comment"]["ancestors"]).to be_present
+    expect(json_data["comment"]["commentable"]).to be_present
+    expect(json_data["comment"]["processed_html"]).to be_present
+  end
+
   it "creates notifications for the article author and the parent comment author" do
     described_class.call(child_comment)
     child_comment.reload