Feature: Improve reports ui

[ThisIsMissEm]

After playing with the improved reports UI from #7000, I still wasn't happy with it. I've since redesigned the form for adding report notes, and added the "ActionLog" history (in a rough form) so we can see all the actions taken in relation to a specific report.

Note: I wasn't sure on the right way to implement fetching the ActionLog, so went with a relation, but now I think a Finder method would be better, as it needs to pull from the Report, the statuses of the report and the reported user; Though I'm not sure on the best way to implement this.

Here's what the page now looks like:

[ThisIsMissEm]

I really want to try to get the additional ActionLog items that are relevant into the report, but I'm not sure how to do this. Open to suggestions.

[nightpool]

it would be really good if the moderation log and action log displayed interleaved with each other I think

…

On Tue, Apr 3, 2018, 7:18 PM Emelia Smith ***@***.***> wrote: I really want to try to get the additional ActionLog items that are relevant into the report, but I'm not sure how to do this. Open to suggestions. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#7032 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAORV-HfERQglSDv-pnbf3XxPz_RA4Tzks5tlAMogaJpZM4TF7g_> .

[ThisIsMissEm]

@nightpool oh, I agree, but I've absolutely no idea how to do that in rails. Not to mention that it's not the full action log for now, which I'm struggling to build.

[Gargron]

I'm liking this a lot. 👍

[ThisIsMissEm]

This probably isn't ideal or perfect, so I'm open to suggestions here @nightpool @Gargron

[ThisIsMissEm]

Okay, I've added a limit on the report history to be only actions within the timeframe of the report (report.created_at..report.updated_at) and ensure that adding notes updates the report.

[Gargron]

It seems that this method takes a block argument, i.e. unprepared_statement { query }, but I am not totally sure why unprepared_statement is needed in here?

[Gargron]

We may want to just use https://github.com/brianhempel/active_record_union to do this the clean way

[ThisIsMissEm]

@Gargron this is just what I found when looking up how to do union queries with rails; there's probably a better way to do this, though I'm a bit out of my depth with rails knowledge needed.

…

On 4 Apr 2018, at 8:06 pm, Eugen Rochko ***@***.***> wrote: @Gargron commented on this pull request. In app/models/report.rb <#7032 (comment)>: > + ).unscope(:order) + + target_account_log = Admin::ActionLog.where( + target_type: 'Account', + target_id: target_account_id, + created_at: created_at..updated_at + ).unscope(:order) + + statuses_log = Admin::ActionLog.where( + target_type: 'Status', + target_id: status_ids, + created_at: created_at..updated_at + ).unscope(:order) + + query = "((#{report_log.to_sql}) UNION ALL (#{target_account_log.to_sql}) UNION ALL (#{statuses_log.to_sql})) as admin_action_logs" + sql = Admin::ActionLog.connection.unprepared_statement(query) It seems that this method takes a block argument, i.e. unprepared_statement { query }, but I am not totally sure why unprepared_statement is needed in here? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#7032 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAB4a5TZrGjAEY2GFCw7UMB-KrK8WERBks5tlQu1gaJpZM4TF7g_>.

[Gargron]

Here is a fix for the UNION stuff that works for me:

diff --git a/app/models/report.rb b/app/models/report.rb
index e1bfa99..5b90c7b 100644
--- a/app/models/report.rb
+++ b/app/models/report.rb
@@ -61,27 +61,28 @@ class Report < ApplicationRecord
   end
 
   def history
-    report_log = Admin::ActionLog.where(
-      target_type: 'Report',
-      target_id: id,
-      created_at: created_at..updated_at
-    ).unscope(:order)
-
-    target_account_log = Admin::ActionLog.where(
-      target_type: 'Account',
-      target_id: target_account_id,
-      created_at: created_at..updated_at
-    ).unscope(:order)
-
-    statuses_log = Admin::ActionLog.where(
-      target_type: 'Status',
-      target_id: status_ids,
-      created_at: created_at..updated_at
-    ).unscope(:order)
-
-    query = "((#{report_log.to_sql}) UNION ALL (#{target_account_log.to_sql}) UNION ALL (#{statuses_log.to_sql})) as admin_action_logs"
-    sql = Admin::ActionLog.connection.unprepared_statement(query)
-
-    Admin::ActionLog.from(sql)
+    time_range = created_at..updated_at
+
+    sql = [
+      Admin::ActionLog.where(
+        target_type: 'Report',
+        target_id: id,
+        created_at: time_range
+      ).unscope(:order),
+
+      Admin::ActionLog.where(
+        target_type: 'Account',
+        target_id: target_account_id,
+        created_at: time_range
+      ).unscope(:order),
+
+      Admin::ActionLog.where(
+        target_type: 'Status',
+        target_id: status_ids,
+        created_at: time_range
+      ).unscope(:order),
+    ].map { |query| "(#{query.to_sql})" }.join(' UNION ALL ')
+
+    Admin::ActionLog.from("(#{sql}) AS admin_action_logs")
   end
 end

[ThisIsMissEm]

@Gargron this is now ready for review again, the 1 issue to fix from code climate isn't an issue, because we know what we're doing™