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)
This commit is contained in:
Krzysztof Kotlarek 2019-11-26 09:17:14 +11:00 committed by GitHub
parent d0ad5ecc6d
commit 6e403f20ee
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 16 additions and 9 deletions

View File

@ -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"]

View File

@ -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