From 6e403f20ee47ff8b7fe2915c79dc920432d8ed72 Mon Sep 17 00:00:00 2001
From: Krzysztof Kotlarek <kotlarek.krzysztof@gmail.com>
Date: Tue, 26 Nov 2019 09:17:14 +1100
Subject: [PATCH] FIX: Don't error when the empty current value in dif (#8406)

If current value is nil we should use `&.` combined with `dig` to protect diff from erroring

It is happening when for example theme is delete (new value is empty)
---
 .../admin/staff_action_logs_controller.rb      | 18 +++++++++---------
 .../admin/staff_action_logs_controller_spec.rb |  7 +++++++
 2 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/app/controllers/admin/staff_action_logs_controller.rb b/app/controllers/admin/staff_action_logs_controller.rb
index 5ab3fbe4d0a..117bdf84152 100644
--- a/app/controllers/admin/staff_action_logs_controller.rb
+++ b/app/controllers/admin/staff_action_logs_controller.rb
@@ -35,23 +35,23 @@ class Admin::StaffActionLogsController < Admin::AdminController
 
     diff_fields = {}
 
-    output = +"<h2>#{CGI.escapeHTML(cur["name"].to_s)}</h2><p></p>"
+    output = +"<h2>#{CGI.escapeHTML(cur&.dig("name").to_s)}</h2><p></p>"
 
     diff_fields["name"] = {
-      prev: prev["name"].to_s,
-      cur: cur["name"].to_s,
+      prev: prev&.dig("name").to_s,
+      cur: cur&.dig("name").to_s,
     }
 
     ["default", "user_selectable"].each do |f|
       diff_fields[f] = {
-        prev: (!!prev[f]).to_s,
-        cur: (!!cur[f]).to_s
+        prev: (!!prev&.dig(f)).to_s,
+        cur: (!!cur&.dig(f)).to_s
       }
     end
 
     diff_fields["color scheme"] = {
-      prev: prev["color_scheme"]&.fetch("name").to_s,
-      cur: cur["color_scheme"]&.fetch("name").to_s,
+      prev: prev&.dig("color_scheme", "name").to_s,
+      cur: cur&.dig("color_scheme", "name").to_s,
     }
 
     diff_fields["included themes"] = {
@@ -76,13 +76,13 @@ class Admin::StaffActionLogsController < Admin::AdminController
   protected
 
   def child_themes(theme)
-    return "" unless children = theme["child_themes"]
+    return "" unless children = theme&.dig("child_themes")
 
     children.map { |row| row["name"] }.join(" ").to_s
   end
 
   def load_diff(hash, key, val)
-    if f = val["theme_fields"]
+    if f = val&.dig("theme_fields")
       f.each do |row|
         entry = hash[row["target"] + " " + row["name"]] ||= {}
         entry[key] = row["value"]
diff --git a/spec/requests/admin/staff_action_logs_controller_spec.rb b/spec/requests/admin/staff_action_logs_controller_spec.rb
index e79c44bbaed..9e639402e4f 100644
--- a/spec/requests/admin/staff_action_logs_controller_spec.rb
+++ b/spec/requests/admin/staff_action_logs_controller_spec.rb
@@ -90,5 +90,12 @@ describe Admin::StaffActionLogsController do
 
       expect(parsed["side_by_side"]).not_to include("omit-dupe")
     end
+
+    it 'is not erroring when current value is empty' do
+      theme = Fabricate(:theme)
+      StaffActionLogger.new(admin).log_theme_destroy(theme)
+      get "/admin/logs/staff_action_logs/#{UserHistory.last.id}/diff.json"
+      expect(response.status).to eq(200)
+    end
   end
 end