From d157073eb872a98f9c697a0e3e959f498a922e12 Mon Sep 17 00:00:00 2001 From: Dave Arter Date: Thu, 23 Apr 2020 15:47:59 +0100 Subject: =?UTF-8?q?Don=E2=80=99t=20show=20`sent=5Fto`=20on=20moderation=20?= =?UTF-8?q?diff?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- perllib/FixMyStreet/DB/Result/ModerationOriginalData.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'perllib/FixMyStreet/DB/Result/ModerationOriginalData.pm') diff --git a/perllib/FixMyStreet/DB/Result/ModerationOriginalData.pm b/perllib/FixMyStreet/DB/Result/ModerationOriginalData.pm index 1805e1fd2..6d14e6a9f 100644 --- a/perllib/FixMyStreet/DB/Result/ModerationOriginalData.pm +++ b/perllib/FixMyStreet/DB/Result/ModerationOriginalData.pm @@ -163,7 +163,7 @@ sub compare_extra { my $new = $other->get_extra_metadata; my $both = { %$old, %$new }; - my @all_keys = sort keys %$both; + my @all_keys = grep { $_ ne 'sent_to' } sort keys %$both; my @s; foreach (@all_keys) { if ($old->{$_} && $new->{$_}) { -- cgit v1.2.3 From ee42ae7994e80e716a694dedcc30ad85333ef98a Mon Sep 17 00:00:00 2001 From: Matthew Somerville Date: Mon, 6 Jul 2020 16:59:28 +0100 Subject: Make sure string_diff does not return undef. As string_diff is used in hash values, it must always return something, otherwise you could end up with e.g. category => 'extra'. --- perllib/FixMyStreet/DB/Result/ModerationOriginalData.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'perllib/FixMyStreet/DB/Result/ModerationOriginalData.pm') diff --git a/perllib/FixMyStreet/DB/Result/ModerationOriginalData.pm b/perllib/FixMyStreet/DB/Result/ModerationOriginalData.pm index 6d14e6a9f..d2059ab9d 100644 --- a/perllib/FixMyStreet/DB/Result/ModerationOriginalData.pm +++ b/perllib/FixMyStreet/DB/Result/ModerationOriginalData.pm @@ -193,7 +193,7 @@ sub string_diff { $new = FixMyStreet::Template::html_filter($new); if ($options{single}) { - return unless $old; + return '' unless $old; $old = [ $old ]; $new = [ $new ]; } -- cgit v1.2.3 From 56ff18a3f9495c41d84cc7c68aea0bc81313b392 Mon Sep 17 00:00:00 2001 From: Matthew Somerville Date: Mon, 6 Jul 2020 17:00:25 +0100 Subject: Handle arrayrefs in moderation extra diff display. --- perllib/FixMyStreet/DB/Result/ModerationOriginalData.pm | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'perllib/FixMyStreet/DB/Result/ModerationOriginalData.pm') diff --git a/perllib/FixMyStreet/DB/Result/ModerationOriginalData.pm b/perllib/FixMyStreet/DB/Result/ModerationOriginalData.pm index d2059ab9d..0ff61d64f 100644 --- a/perllib/FixMyStreet/DB/Result/ModerationOriginalData.pm +++ b/perllib/FixMyStreet/DB/Result/ModerationOriginalData.pm @@ -166,6 +166,8 @@ sub compare_extra { my @all_keys = grep { $_ ne 'sent_to' } sort keys %$both; my @s; foreach (@all_keys) { + $old->{$_} = join(', ', @{$old->{$_}}) if ref $old->{$_} eq 'ARRAY'; + $new->{$_} = join(', ', @{$new->{$_}}) if ref $new->{$_} eq 'ARRAY'; if ($old->{$_} && $new->{$_}) { push @s, string_diff("$_ = $old->{$_}", "$_ = $new->{$_}"); } elsif ($new->{$_}) { @@ -174,7 +176,7 @@ sub compare_extra { push @s, string_diff("$_ = $old->{$_}", ""); } } - return join ', ', grep { $_ } @s; + return join '; ', grep { $_ } @s; } sub extra_diff { -- cgit v1.2.3 From 743c60c5e5d134d82533526758b4411198efe063 Mon Sep 17 00:00:00 2001 From: Matthew Somerville Date: Mon, 6 Jul 2020 17:00:48 +0100 Subject: Only show removed in moderation diff if non-blank. --- perllib/FixMyStreet/DB/Result/ModerationOriginalData.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'perllib/FixMyStreet/DB/Result/ModerationOriginalData.pm') diff --git a/perllib/FixMyStreet/DB/Result/ModerationOriginalData.pm b/perllib/FixMyStreet/DB/Result/ModerationOriginalData.pm index 0ff61d64f..b9abb4e40 100644 --- a/perllib/FixMyStreet/DB/Result/ModerationOriginalData.pm +++ b/perllib/FixMyStreet/DB/Result/ModerationOriginalData.pm @@ -172,7 +172,7 @@ sub compare_extra { push @s, string_diff("$_ = $old->{$_}", "$_ = $new->{$_}"); } elsif ($new->{$_}) { push @s, string_diff("", "$_ = $new->{$_}"); - } else { + } elsif ($old->{$_}) { push @s, string_diff("$_ = $old->{$_}", ""); } } -- cgit v1.2.3 From 142524462031ff382f122d689bb38908149267b0 Mon Sep 17 00:00:00 2001 From: Matthew Somerville Date: Mon, 6 Jul 2020 17:02:11 +0100 Subject: Provide list of keys to ignore in moderation diff. This is a current full list of extra keys that could be set on a report that could confuse the moderation diff display. --- perllib/FixMyStreet/DB/Result/ModerationOriginalData.pm | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) (limited to 'perllib/FixMyStreet/DB/Result/ModerationOriginalData.pm') diff --git a/perllib/FixMyStreet/DB/Result/ModerationOriginalData.pm b/perllib/FixMyStreet/DB/Result/ModerationOriginalData.pm index b9abb4e40..dd76a52c0 100644 --- a/perllib/FixMyStreet/DB/Result/ModerationOriginalData.pm +++ b/perllib/FixMyStreet/DB/Result/ModerationOriginalData.pm @@ -156,6 +156,20 @@ sub compare_photo { return FixMyStreet::Template::SafeString->new($s); } +# This is a list of extra keys that could be set on a report after a moderation +# has occurred. This can confuse the display of the last moderation entry, as +# the comparison with the problem's extra will be wrong. +my @keys_to_ignore = ( + 'sent_to', # SendReport::Email adds this arrayref when sent + 'closed_updates', # Marked to close a report to updates + 'closure_alert_sent_at', # Set by alert sending if update closes a report + # Can be set/changed by an Open311 update + 'external_status_code', 'customer_reference', + # Can be set by inspectors + 'traffic_information', 'detailed_information', 'duplicates', 'duplicate_of', 'order', +); +my %keys_to_ignore = map { $_ => 1 } @keys_to_ignore; + sub compare_extra { my ($self, $other) = @_; @@ -163,7 +177,7 @@ sub compare_extra { my $new = $other->get_extra_metadata; my $both = { %$old, %$new }; - my @all_keys = grep { $_ ne 'sent_to' } sort keys %$both; + my @all_keys = grep { !$keys_to_ignore{$_} } sort keys %$both; my @s; foreach (@all_keys) { $old->{$_} = join(', ', @{$old->{$_}}) if ref $old->{$_} eq 'ARRAY'; -- cgit v1.2.3