From 5d5d196ef1fdec2bb5c2d444da0126ecb5adeb78 Mon Sep 17 00:00:00 2001 From: Dave Arter Date: Tue, 15 Nov 2016 15:14:25 +0000 Subject: =?UTF-8?q?Display=20nearby=20duplicate=20reports=20when=20setting?= =?UTF-8?q?=20report=20category=20to=20=E2=80=98duplicate=E2=80=99?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- templates/web/base/report/_inspect.html | 5 +++++ web/cobrands/fixmystreet/fixmystreet.js | 34 +++++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+) diff --git a/templates/web/base/report/_inspect.html b/templates/web/base/report/_inspect.html index 012411b7e..a09d75d40 100644 --- a/templates/web/base/report/_inspect.html +++ b/templates/web/base/report/_inspect.html @@ -76,6 +76,11 @@ [% END %]

+ [% END %] diff --git a/web/cobrands/fixmystreet/fixmystreet.js b/web/cobrands/fixmystreet/fixmystreet.js index 1701c5cd0..a7e6f28f0 100644 --- a/web/cobrands/fixmystreet/fixmystreet.js +++ b/web/cobrands/fixmystreet/fixmystreet.js @@ -391,6 +391,40 @@ $.extend(fixmystreet.set_up, { }, + state_change: function() { + // Deal with changes to report state by inspector/other staff, specifically + // displaying nearby reports if it's changed to 'duplicate'. + $("#report_inspect_form").on("change.state", "select#state", function() { + var state = $(this).val(); + + if (state !== "duplicate") { + $("#js-duplicate-reports").addClass("hidden"); + $("#js-duplicate-reports ul").empty(); + return; + } + + var args = { + state: state, + filter_category: $("#report_inspect_form select#category").val() + }; + var bounds = fixmystreet.map.getExtent(); + bounds.transform(bounds.transform(fixmystreet.map.getProjectionObject(), new OpenLayers.Projection("EPSG:4326"))); + args.bbox = bounds.toBBOX(); + + args.latitude = $('input[name="latitude"]').val(); + args.longitude = $('input[name="longitude"]').val(); + + console.log(args); + + $.getJSON('/ajax', args, function(data) { + var $reports = $(data.current); + $("#js-duplicate-reports").removeClass("hidden"); + $reports.slice(0, 5).appendTo($("#js-duplicate-reports ul").empty()); + }); + }); + }, + + contribute_as: function() { $('.content').on('change', '.js-contribute-as', function(){ var opt = this.options[this.selectedIndex], -- cgit v1.2.3 From dc151edce28d2e5bf873f8e86473a82b9c6f178a Mon Sep 17 00:00:00 2001 From: Dave Arter Date: Tue, 15 Nov 2016 17:08:57 +0000 Subject: Store and display selected duplicate report --- perllib/FixMyStreet/App/Controller/Report.pm | 2 +- perllib/FixMyStreet/DB/Result/Problem.pm | 11 +++++++++++ templates/web/base/report/_inspect.html | 11 ++++++++--- templates/web/base/report/_item.html | 2 +- web/cobrands/fixmystreet/fixmystreet.js | 21 +++++++++++++++++---- web/cobrands/sass/_report_list_pins.scss | 8 ++++++++ 6 files changed, 46 insertions(+), 9 deletions(-) diff --git a/perllib/FixMyStreet/App/Controller/Report.pm b/perllib/FixMyStreet/App/Controller/Report.pm index 13967601b..a9d171d33 100644 --- a/perllib/FixMyStreet/App/Controller/Report.pm +++ b/perllib/FixMyStreet/App/Controller/Report.pm @@ -320,7 +320,7 @@ sub inspect : Private { my $reputation_change = 0; if ($permissions->{report_inspect}) { - foreach (qw/detailed_information traffic_information/) { + foreach (qw/detailed_information traffic_information duplicate_of/) { $problem->set_extra_metadata( $_ => $c->get_param($_) ); } diff --git a/perllib/FixMyStreet/DB/Result/Problem.pm b/perllib/FixMyStreet/DB/Result/Problem.pm index ab6f20050..c63e6c881 100644 --- a/perllib/FixMyStreet/DB/Result/Problem.pm +++ b/perllib/FixMyStreet/DB/Result/Problem.pm @@ -1032,4 +1032,15 @@ has shortlisted_user => ( }, ); +has duplicate_of => ( + is => 'ro', + lazy => 1, + default => sub { + my $self = shift; + my $duplicate_of = $self->get_extra_metadata("duplicate_of"); + return unless defined $duplicate_of; + return $self->result_source->schema->resultset('Problem')->search({ id => $duplicate_of })->first; + }, +); + 1; diff --git a/templates/web/base/report/_inspect.html b/templates/web/base/report/_inspect.html index a09d75d40..d5df3d42e 100644 --- a/templates/web/base/report/_inspect.html +++ b/templates/web/base/report/_inspect.html @@ -10,6 +10,7 @@

[% loc('Report ID:') %] [% problem.id %] +

[% SET local_coords = problem.local_coords; %] @@ -76,10 +77,14 @@ [% END %]

- -- cgit v1.2.3 From 2906f139d37b308e071fc8ed388f87fa45ce7b00 Mon Sep 17 00:00:00 2001 From: Dave Arter Date: Thu, 17 Nov 2016 10:58:12 +0000 Subject: Better preserve chosen duplicate report if state is changed --- templates/web/base/report/_inspect.html | 6 ++++-- web/cobrands/fixmystreet/fixmystreet.js | 12 +++++++++--- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/templates/web/base/report/_inspect.html b/templates/web/base/report/_inspect.html index 3479058c3..655c70128 100644 --- a/templates/web/base/report/_inspect.html +++ b/templates/web/base/report/_inspect.html @@ -79,10 +79,12 @@

- [% loc('Which report is it a duplicate of?') %] +

[% loc('Duplicate of') %]

+

[% loc('Which report is it a duplicate of?') %]
diff --git a/web/cobrands/fixmystreet/fixmystreet.js b/web/cobrands/fixmystreet/fixmystreet.js index 1fc17ac9b..22e021edf 100644 --- a/web/cobrands/fixmystreet/fixmystreet.js +++ b/web/cobrands/fixmystreet/fixmystreet.js @@ -399,7 +399,6 @@ $.extend(fixmystreet.set_up, { if (state !== "duplicate") { $("#js-duplicate-reports").addClass("hidden"); - $("#js-duplicate-reports ul").empty(); return; } @@ -414,16 +413,23 @@ $.extend(fixmystreet.set_up, { args.latitude = $('input[name="latitude"]').val(); args.longitude = $('input[name="longitude"]').val(); + $("#js-duplicate-reports").removeClass("hidden"); + + var duplicate_of = $("#report_inspect_form [name=duplicate_of]").val(); + if (!!duplicate_of) { + return; + } + + $("#js-duplicate-reports ul").html("
  • Loading...
    • "); + var report_id = $("#report_inspect_form [name=report_id]").val(); $.getJSON('/report/'+report_id+'/nearby', args, function(data) { - var duplicate_of = $("#report_inspect_form [name=duplicate_of]").val(); var $reports = $(data.current) .filter("li") .not("[data-report-id="+report_id+"]") .slice(0, 5); $reports.filter("[data-report-id="+duplicate_of+"]").addClass("item-list--reports__item--selected"); - $("#js-duplicate-reports").removeClass("hidden"); $("#js-duplicate-reports ul").empty().prepend($reports); $reports.find("a").click(function() { -- cgit v1.2.3 From 21384a0712877fd20f17552b1b4a68b0fd6c8eb5 Mon Sep 17 00:00:00 2001 From: Dave Arter Date: Mon, 21 Nov 2016 15:18:03 +0000 Subject: Refactor JS to work with ajax-loaded reports --- templates/web/base/report/_inspect.html | 3 +- web/cobrands/fixmystreet/fixmystreet.js | 96 +++++++++++++++++---------------- 2 files changed, 50 insertions(+), 49 deletions(-) diff --git a/templates/web/base/report/_inspect.html b/templates/web/base/report/_inspect.html index 655c70128..7f41a2d63 100644 --- a/templates/web/base/report/_inspect.html +++ b/templates/web/base/report/_inspect.html @@ -9,8 +9,7 @@

    [% loc('Report ID:') %] - [% problem.id %] - + [% problem.id %]

    [% SET local_coords = problem.local_coords; %] diff --git a/web/cobrands/fixmystreet/fixmystreet.js b/web/cobrands/fixmystreet/fixmystreet.js index 22e021edf..b314bc991 100644 --- a/web/cobrands/fixmystreet/fixmystreet.js +++ b/web/cobrands/fixmystreet/fixmystreet.js @@ -391,56 +391,57 @@ $.extend(fixmystreet.set_up, { }, - state_change: function() { - // Deal with changes to report state by inspector/other staff, specifically - // displaying nearby reports if it's changed to 'duplicate'. - $("#report_inspect_form").on("change.state", "select#state", function() { - var state = $(this).val(); - - if (state !== "duplicate") { - $("#js-duplicate-reports").addClass("hidden"); - return; - } - - var args = { - state: state, - filter_category: $("#report_inspect_form select#category").val() - }; - var bounds = fixmystreet.map.getExtent(); - bounds.transform(bounds.transform(fixmystreet.map.getProjectionObject(), new OpenLayers.Projection("EPSG:4326"))); - args.bbox = bounds.toBBOX(); - - args.latitude = $('input[name="latitude"]').val(); - args.longitude = $('input[name="longitude"]').val(); - - $("#js-duplicate-reports").removeClass("hidden"); - - var duplicate_of = $("#report_inspect_form [name=duplicate_of]").val(); - if (!!duplicate_of) { - return; - } - - $("#js-duplicate-reports ul").html("

  • Loading...
    • "); + manage_duplicates: function() { + // Deal with changes to report state by inspector/other staff, specifically + // displaying nearby reports if it's changed to 'duplicate'. + function refresh_duplicate_list() { + var report_id = $("#report_inspect_form .js-report-id").text(); + var args = { + filter_category: $("#report_inspect_form select#category").val(), + latitude: $('input[name="latitude"]').val(), + longitude: $('input[name="longitude"]').val() + }; + $("#js-duplicate-reports ul").html("
  • Loading...
    • "); + $.getJSON('/report/'+report_id+'/nearby', args, function(data) { + var duplicate_of = $("#report_inspect_form [name=duplicate_of]").val(); + var $reports = $(data.current) + .filter("li") + .not("[data-report-id="+report_id+"]") + .slice(0, 5); + $reports.filter("[data-report-id="+duplicate_of+"]").addClass("item-list--reports__item--selected"); + + $("#js-duplicate-reports ul").empty().prepend($reports); + + $reports.find("a").click(function() { + var report_id = $(this).closest("li").data('reportId'); + $("#report_inspect_form [name=duplicate_of]").val(report_id); + $("#js-duplicate-reports ul li").removeClass("item-list--reports__item--selected"); + $(this).closest("li").addClass("item-list--reports__item--selected"); + return false; + }); + }); + } - var report_id = $("#report_inspect_form [name=report_id]").val(); - $.getJSON('/report/'+report_id+'/nearby', args, function(data) { - var $reports = $(data.current) - .filter("li") - .not("[data-report-id="+report_id+"]") - .slice(0, 5); - $reports.filter("[data-report-id="+duplicate_of+"]").addClass("item-list--reports__item--selected"); + function state_change() { + // The duplicate report list only makes sense when state is 'duplicate' + if ($(this).val() !== "duplicate") { + $("#js-duplicate-reports").addClass("hidden"); + return; + } else { + $("#js-duplicate-reports").removeClass("hidden"); + } + // If this report is already marked as a duplicate of another, then + // there's no need to refresh the list of duplicate reports + var duplicate_of = $("#report_inspect_form [name=duplicate_of]").val(); + if (!!duplicate_of) { + return; + } - $("#js-duplicate-reports ul").empty().prepend($reports); + refresh_duplicate_list(); + } - $reports.find("a").click(function() { - var report_id = $(this).closest("li").data('reportId'); - $("#report_inspect_form [name=duplicate_of]").val(report_id); - $("#js-duplicate-reports ul li").removeClass("item-list--reports__item--selected"); - $(this).closest("li").addClass("item-list--reports__item--selected"); - return false; - }); - }); - }); + $("#report_inspect_form").on("change.state", "select#state", state_change); + $("#js-change-duplicate-report").click(refresh_duplicate_list); }, @@ -1146,6 +1147,7 @@ fixmystreet.display = { $twoColReport.appendTo('#map_sidebar'); $('body').addClass('with-actions'); fixmystreet.set_up.report_page_inspect(); + fixmystreet.set_up.manage_duplicates(); } else { $sideReport.appendTo('#map_sidebar'); } -- cgit v1.2.3 From b377665d0d839825afa65e7393cfedb13ae85e80 Mon Sep 17 00:00:00 2001 From: Dave Arter Date: Mon, 21 Nov 2016 16:31:00 +0000 Subject: Signpost users to original version of duplicate reports This stops updates being left on duplicates. --- t/app/controller/report_display.t | 50 +++---- templates/web/base/report/_inspect.html | 4 +- templates/web/base/report/display.html | 9 +- .../web/base/report/duplicate-no-updates.html | 7 + templates/web/bromley/report/display.html | 148 ++------------------- templates/web/bromley/report/update-form.html | 137 +++++++++++++++++++ 6 files changed, 190 insertions(+), 165 deletions(-) create mode 100644 templates/web/base/report/duplicate-no-updates.html create mode 100644 templates/web/bromley/report/update-form.html diff --git a/t/app/controller/report_display.t b/t/app/controller/report_display.t index fb532ddc4..9817a640d 100644 --- a/t/app/controller/report_display.t +++ b/t/app/controller/report_display.t @@ -25,31 +25,16 @@ my $dt = DateTime->new( second => 23 ); -my $report = FixMyStreet::App->model('DB::Problem')->find_or_create( - { - postcode => 'SW1A 1AA', - bodies_str => '2504', - areas => ',105255,11806,11828,2247,2504,', - category => 'Other', - title => 'Test 2', - detail => 'Test 2 Detail', - used_map => 't', - name => 'Test User', - anonymous => 'f', - state => 'confirmed', - confirmed => $dt->ymd . ' ' . $dt->hms, - lang => 'en-gb', - service => '', - cobrand => 'default', - cobrand_data => '', - send_questionnaire => 't', - latitude => '51.5016605453401', - longitude => '-0.142497580865087', - user_id => $user->id, - } -); +my $westminster = $mech->create_body_ok(2504, 'Westminster City Council'); +my ($report, $report2) = $mech->create_problems_for_body(2, $westminster->id, "Example", { + user => $user, + confirmed => $dt->ymd . ' ' . $dt->hms, +}); +$report->update({ + title => 'Test 2', + detail => 'Test 2 Detail' +}); my $report_id = $report->id; -ok $report, "created test report - $report_id"; subtest "check that no id redirects to homepage" => sub { $mech->get_ok('/report'); @@ -125,6 +110,22 @@ subtest "check owner of report can view non public reports" => sub { ok $report->update( { non_public => 0 } ), 'make report public'; }; +subtest "duplicate reports are signposted correctly" => sub { + $report2->set_extra_metadata(duplicate_of => $report->id); + $report2->state('duplicate'); + $report2->update; + + my $report2_id = $report2->id; + ok $mech->get("/report/$report2_id"), "get '/report/$report2_id'"; + $mech->content_contains('This report is a duplicate.'); + $mech->content_contains($report->title); + $mech->log_out_ok; + + $report2->unset_extra_metadata('duplicate_of'); + $report2->state('confirmed'); + $report2->update; +}; + subtest "test a good report" => sub { $mech->get_ok("/report/$report_id"); is $mech->uri->path, "/report/$report_id", "at /report/$report_id"; @@ -532,5 +533,6 @@ subtest "Zurich banners are displayed correctly" => sub { END { $mech->delete_user('test@example.com'); + $mech->delete_body($westminster); done_testing(); } diff --git a/templates/web/base/report/_inspect.html b/templates/web/base/report/_inspect.html index 7f41a2d63..6f49f3ecb 100644 --- a/templates/web/base/report/_inspect.html +++ b/templates/web/base/report/_inspect.html @@ -80,7 +80,7 @@

    [% loc('Duplicate of') %]

    [% loc('Which report is it a duplicate of?') %]

    -
      +
        [% IF problem.duplicate_of %] [% INCLUDE 'report/_item.html' item_extra_class = 'item-list--reports__item--selected' problem = problem.duplicate_of %]
      • [% loc('Choose another') %]
        • @@ -89,7 +89,7 @@
    [% IF problem.duplicates.size %]

    [% loc('Duplicates') %]

    -
      +
        [% FOR duplicate IN problem.duplicates %] [% INCLUDE 'report/_item.html' problem = duplicate %] [% END %] diff --git a/templates/web/base/report/display.html b/templates/web/base/report/display.html index 1ee5c4636..7c26c4938 100644 --- a/templates/web/base/report/display.html +++ b/templates/web/base/report/display.html @@ -40,12 +40,19 @@ [% INCLUDE 'report/banner.html' %] [% INCLUDE 'report/_main.html' %] + +[% IF problem.duplicate_of %] + [% INCLUDE 'report/duplicate-no-updates.html' hide_header = 1 %] +[% END %] + [% TRY %][% INCLUDE 'report/_message_manager.html' %][% CATCH file %][% END %] [% INCLUDE 'report/display_tools.html' %] [% TRY %][% INCLUDE 'report/sharing.html' %][% CATCH file %][% END %] [% INCLUDE 'report/updates.html' %] -[% IF NOT shown_form %] +[% IF problem.duplicate_of %] + [% INCLUDE 'report/duplicate-no-updates.html' %] +[% ELSIF NOT shown_form %] [% INCLUDE 'report/update-form.html' %] [% END %] diff --git a/templates/web/base/report/duplicate-no-updates.html b/templates/web/base/report/duplicate-no-updates.html new file mode 100644 index 000000000..eb9ded728 --- /dev/null +++ b/templates/web/base/report/duplicate-no-updates.html @@ -0,0 +1,7 @@ +
        + [% UNLESS hide_header %]

        [% loc( 'Provide an update') %]

        [% END %] +

        [% loc("This report is a duplicate. Please leave updates on the original report:") %]

        +
          + [% INCLUDE 'report/_item.html' item_extra_class = 'item-list__item--with-pin item-list--reports__item--selected' problem = problem.duplicate_of %] +
        +
        diff --git a/templates/web/bromley/report/display.html b/templates/web/bromley/report/display.html index 9f2089d28..4c1a69bca 100644 --- a/templates/web/bromley/report/display.html +++ b/templates/web/bromley/report/display.html @@ -21,147 +21,19 @@ [% INCLUDE 'report/banner.html' %] [% INCLUDE 'report/_main.html' %] -[% INCLUDE 'report/display_tools.html' %] -[% INCLUDE 'report/updates.html' %] - -
        -

        [% loc( 'Provide an update') %]

        - - [% INCLUDE 'errors.html' %] - -
        - -
        - - - - [% IF c.cobrand.allow_photo_upload %] - - - - [% IF field_errors.photo %] -

        [% field_errors.photo %]

        - [% END %] - -
        - [% IF upload_fileid %] -

        [% loc('You have already attached photos to this update. Note that you can attach a maximum of 3 to this update (if you try to upload more, the oldest will be removed).') %]

        - [% FOREACH id IN upload_fileid.split(',') %] - - [% END %] - [% END %] - - - - - -
        - [% END %] - - - [% IF field_errors.update %] -
        [% field_errors.update %]
        - [% END %] - - -
        -

        Please note this comments box can only be used for this report. -
        Report a different issue -

        - - [% IF c.user && c.user.belongs_to_body( problem.bodies_str ) %] - - - [% ELSE %] - [% IF problem.is_fixed AND c.user_exists AND c.user.id == problem.user_id %] - - - - - [% ELSIF !problem.is_fixed %] - -
        - - -
        - - [% END %] - [% END %] - - [% IF c.user_exists %] - - [% INCLUDE name %] - - +[% IF problem.duplicate_of %] + [% INCLUDE 'report/duplicate-no-updates.html' hide_header = 1 %] +[% END %] - [% ELSE %] - - - - [% IF field_errors.email %] -

        [% field_errors.email %]

        - [% END %] - - -
        -

        To submit your update you now need to confirm it either by email or by using a FixMyStreet password.

        - -
        -
        Confirm my report by email
        - - [% INCLUDE name %] - - - -
        -

        [% loc('Providing a password is optional, but doing so will allow you to more easily report problems, leave updates and manage your reports.') %]

        -
        - -
        - - -
        -
        -
        -
        Confirm my report with my FixMyStreet password
        - - - [% IF field_errors.password %] -

        [% field_errors.password %]

        - [% END %] -
        - - -
        - -
        - - -
        -
        -
        - - [% END %] - -

        Your information will only be used in accordance with our privacy policy.

        - -
        -
        -
        +[% INCLUDE 'report/display_tools.html' %] +[% INCLUDE 'report/updates.html' %] +[% IF problem.duplicate_of %] + [% INCLUDE 'report/duplicate-no-updates.html' %] +[% ELSE %] + [% INCLUDE 'report/update-form.html' %] +[% END %] [% INCLUDE 'footer.html' %] diff --git a/templates/web/bromley/report/update-form.html b/templates/web/bromley/report/update-form.html new file mode 100644 index 000000000..112cb2960 --- /dev/null +++ b/templates/web/bromley/report/update-form.html @@ -0,0 +1,137 @@ +
        +

        [% loc( 'Provide an update') %]

        + + [% INCLUDE 'errors.html' %] + +
        + +
        + + + + [% IF c.cobrand.allow_photo_upload %] + + + + [% IF field_errors.photo %] +

        [% field_errors.photo %]

        + [% END %] + +
        + [% IF upload_fileid %] +

        [% loc('You have already attached photos to this update. Note that you can attach a maximum of 3 to this update (if you try to upload more, the oldest will be removed).') %]

        + [% FOREACH id IN upload_fileid.split(',') %] + + [% END %] + [% END %] + + + + + +
        + [% END %] + + + [% IF field_errors.update %] +
        [% field_errors.update %]
        + [% END %] + + +
        +

        Please note this comments box can only be used for this report. +
        Report a different issue +

        + + [% IF c.user && c.user.belongs_to_body( problem.bodies_str ) %] + + + [% ELSE %] + [% IF problem.is_fixed AND c.user_exists AND c.user.id == problem.user_id %] + + + + + [% ELSIF !problem.is_fixed %] + +
        + + +
        + + [% END %] + [% END %] + + [% IF c.user_exists %] + + [% INCLUDE name %] + + + + + [% ELSE %] + + + + [% IF field_errors.email %] +

        [% field_errors.email %]

        + [% END %] + + +
        +

        To submit your update you now need to confirm it either by email or by using a FixMyStreet password.

        + +
        +
        Confirm my report by email
        + + [% INCLUDE name %] + + + +
        +

        [% loc('Providing a password is optional, but doing so will allow you to more easily report problems, leave updates and manage your reports.') %]

        +
        + +
        + + +
        +
        +
        +
        Confirm my report with my FixMyStreet password
        + + + [% IF field_errors.password %] +

        [% field_errors.password %]

        + [% END %] +
        + + +
        + +
        + + +
        +
        +
        + + [% END %] + +

        Your information will only be used in accordance with our privacy policy.

        + +
        +
        +
        -- cgit v1.2.3 From 05ba5147de9dc7b68f3c9048771fcabf80f20eca Mon Sep 17 00:00:00 2001 From: Dave Arter Date: Thu, 24 Nov 2016 13:56:00 +0000 Subject: Display nearby candidate reports when marking as duplicate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Use Problem->pin_data for single report page - Promote markers_highlight to fixmystreet.maps API We want to highlight map pins on the duplicate report selection UI, so let's use what's already there instead of writing something new. - Make sure duplicate report pins aren’t draggable --- perllib/FixMyStreet/App/Controller/Report.pm | 20 ++++--- perllib/FixMyStreet/DB/Result/Problem.pm | 1 + web/cobrands/fixmystreet/fixmystreet.js | 30 +++++++++- web/js/map-OpenLayers.js | 86 ++++++++++++++++------------ web/js/map-google.js | 3 + 5 files changed, 94 insertions(+), 46 deletions(-) diff --git a/perllib/FixMyStreet/App/Controller/Report.pm b/perllib/FixMyStreet/App/Controller/Report.pm index f9eaf583c..154f0ac94 100644 --- a/perllib/FixMyStreet/App/Controller/Report.pm +++ b/perllib/FixMyStreet/App/Controller/Report.pm @@ -231,13 +231,8 @@ sub generate_map_tags : Private { latitude => $problem->latitude, longitude => $problem->longitude, pins => $problem->used_map - ? [ { - latitude => $problem->latitude, - longitude => $problem->longitude, - colour => $c->cobrand->pin_colour($problem, 'report'), - type => 'big', - } ] - : [], + ? [ $problem->pin_data($c, 'report', type => 'big') ] + : [], ); return 1; @@ -431,14 +426,21 @@ sub nearby_json : Private { my $nearby = $c->model('DB::Nearby')->nearby( $c, $dist, [ $p->id ], 5, $p->latitude, $p->longitude, undef, [ $p->category ], undef ); - # my @reports = map { $_->problem : $_; } @$nearby; + my @pins = map { + my $p = $_->problem; + my $colour = $c->cobrand->pin_colour( $p, 'around' ); + [ $p->latitude, $p->longitude, + $colour, + $p->id, $p->title_safe, 'small', JSON->false + ] + } @$nearby; my $on_map_list_html = $c->render_fragment( 'around/on_map_list_items.html', { on_map => [], around_map => $nearby } ); - my $json = {}; + my $json = { pins => \@pins }; $json->{current} = $on_map_list_html if $on_map_list_html; my $body = encode_json($json); $c->res->content_type('application/json; charset=utf-8'); diff --git a/perllib/FixMyStreet/DB/Result/Problem.pm b/perllib/FixMyStreet/DB/Result/Problem.pm index b7573d994..ec1534fe9 100644 --- a/perllib/FixMyStreet/DB/Result/Problem.pm +++ b/perllib/FixMyStreet/DB/Result/Problem.pm @@ -942,6 +942,7 @@ sub pin_data { id => $self->id, title => $opts{private} ? $self->title : $self->title_safe, problem => $self, + type => $opts{type}, } }; diff --git a/web/cobrands/fixmystreet/fixmystreet.js b/web/cobrands/fixmystreet/fixmystreet.js index b314bc991..2a5c85872 100644 --- a/web/cobrands/fixmystreet/fixmystreet.js +++ b/web/cobrands/fixmystreet/fixmystreet.js @@ -402,7 +402,8 @@ $.extend(fixmystreet.set_up, { longitude: $('input[name="longitude"]').val() }; $("#js-duplicate-reports ul").html("
      • Loading...
        • "); - $.getJSON('/report/'+report_id+'/nearby', args, function(data) { + var nearby_url = '/report/'+report_id+'/nearby.json'; + $.getJSON(nearby_url, args, function(data) { var duplicate_of = $("#report_inspect_form [name=duplicate_of]").val(); var $reports = $(data.current) .filter("li") @@ -410,6 +411,16 @@ $.extend(fixmystreet.set_up, { .slice(0, 5); $reports.filter("[data-report-id="+duplicate_of+"]").addClass("item-list--reports__item--selected"); + (function() { + var timeout; + $reports.on('mouseenter', function(){ + clearTimeout(timeout); + fixmystreet.maps.markers_highlight(parseInt($(this).data('reportId'), 10)); + }).on('mouseleave', function(){ + timeout = setTimeout(fixmystreet.maps.markers_highlight, 50); + }); + })(); + $("#js-duplicate-reports ul").empty().prepend($reports); $reports.find("a").click(function() { @@ -419,9 +430,26 @@ $.extend(fixmystreet.set_up, { $(this).closest("li").addClass("item-list--reports__item--selected"); return false; }); + + show_nearby_pins(data, report_id); }); } + function show_nearby_pins(data, report_id) { + var markers = fixmystreet.maps.markers_list( data.pins, true ); + // We're replacing all the features in the markers layer with the + // possible duplicates, but the list of pins from the server doesn't + // include the current report. So we need to extract the feature for + // the current report and include it in the list of features we're + // showing on the layer. + var report_marker = fixmystreet.maps.get_marker_by_id(parseInt(report_id, 10)); + if (report_marker) { + markers.unshift(report_marker); + } + fixmystreet.markers.removeAllFeatures(); + fixmystreet.markers.addFeatures( markers ); + } + function state_change() { // The duplicate report list only makes sense when state is 'duplicate' if ($(this).val() !== "duplicate") { diff --git a/web/js/map-OpenLayers.js b/web/js/map-OpenLayers.js index a3cefa7da..49801911b 100644 --- a/web/js/map-OpenLayers.js +++ b/web/js/map-OpenLayers.js @@ -90,7 +90,8 @@ var fixmystreet = fixmystreet || {}; size: pin[5] || marker_size, faded: 0, id: pin[3], - title: pin[4] || '' + title: pin[4] || '', + draggable: pin[6] === false ? false : true }); markers.push( marker ); } @@ -144,7 +145,7 @@ var fixmystreet = fixmystreet || {}; admin_drag: function(pin_moved_callback, confirm_change) { confirm_change = confirm_change || false; var original_lonlat; - var drag = new OpenLayers.Control.DragFeature( fixmystreet.markers, { + var drag = new OpenLayers.Control.DragFeatureFMS( fixmystreet.markers, { onStart: function(feature, e) { // Keep track of where the feature started, so we can put it // back if the user cancels the operation. @@ -167,12 +168,41 @@ var fixmystreet = fixmystreet || {}; } ); fixmystreet.map.addControl( drag ); drag.activate(); + }, + + // `markers.redraw()` in markers_highlight will trigger an + // `overFeature` event if the mouse cursor is still over the same + // marker on the map, which would then run markers_highlight + // again, causing an infinite flicker while the cursor remains over + // the same marker. We really only want to redraw the markers when + // the cursor moves from one marker to another (ie: when there is an + // overFeature followed by an outFeature followed by an overFeature). + // Therefore, we keep track of the previous event in + // fixmystreet.latest_map_hover_event and only call markers_highlight + // if we know the previous event was different to the current one. + // (See the `overFeature` and `outFeature` callbacks inside of + // fixmystreet.select_feature). + + markers_highlight: function(problem_id) { + for (var i = 0; i < fixmystreet.markers.features.length; i++) { + if (typeof problem_id == 'undefined') { + // There is no highlighted marker, so unfade this marker + fixmystreet.markers.features[i].attributes.faded = 0; + } else if (problem_id == fixmystreet.markers.features[i].attributes.id) { + // This is the highlighted marker, unfade it + fixmystreet.markers.features[i].attributes.faded = 0; + } else { + // This is not the hightlighted marker, fade it + fixmystreet.markers.features[i].attributes.faded = 1; + } + } + fixmystreet.markers.redraw(); } }; var drag = { activate: function() { - this._drag = new OpenLayers.Control.DragFeature( fixmystreet.markers, { + this._drag = new OpenLayers.Control.DragFeatureFMS( fixmystreet.markers, { onComplete: function(feature, e) { fixmystreet.update_pin( feature.geometry ); } @@ -195,35 +225,6 @@ var fixmystreet = fixmystreet || {}; fixmystreet.map.setCenter(center, z); } - // `markers.redraw()` in markers_highlight will trigger an - // `overFeature` event if the mouse cursor is still over the same - // marker on the map, which would then run markers_highlight - // again, causing an infinite flicker while the cursor remains over - // the same marker. We really only want to redraw the markers when - // the cursor moves from one marker to another (ie: when there is an - // overFeature followed by an outFeature followed by an overFeature). - // Therefore, we keep track of the previous event in - // fixmystreet.latest_map_hover_event and only call markers_highlight - // if we know the previous event was different to the current one. - // (See the `overFeature` and `outFeature` callbacks inside of - // fixmystreet.select_feature). - - function markers_highlight(problem_id) { - for (var i = 0; i < fixmystreet.markers.features.length; i++) { - if (typeof problem_id == 'undefined') { - // There is no highlighted marker, so unfade this marker - fixmystreet.markers.features[i].attributes.faded = 0; - } else if (problem_id == fixmystreet.markers.features[i].attributes.id) { - // This is the highlighted marker, unfade it - fixmystreet.markers.features[i].attributes.faded = 0; - } else { - // This is not the hightlighted marker, fade it - fixmystreet.markers.features[i].attributes.faded = 1; - } - } - fixmystreet.markers.redraw(); - } - function sidebar_highlight(problem_id) { if (typeof problem_id !== 'undefined') { var $a = $('.item-list--reports a[href$="/' + problem_id + '"]'); @@ -505,7 +506,7 @@ var fixmystreet = fixmystreet || {}; overFeature: function (feature) { if (fixmystreet.latest_map_hover_event != 'overFeature') { document.getElementById('map').style.cursor = 'pointer'; - markers_highlight(feature.attributes.id); + fixmystreet.maps.markers_highlight(feature.attributes.id); sidebar_highlight(feature.attributes.id); fixmystreet.latest_map_hover_event = 'overFeature'; } @@ -513,7 +514,7 @@ var fixmystreet = fixmystreet || {}; outFeature: function (feature) { if (fixmystreet.latest_map_hover_event != 'outFeature') { document.getElementById('map').style.cursor = ''; - markers_highlight(); + fixmystreet.maps.markers_highlight(); sidebar_highlight(); fixmystreet.latest_map_hover_event = 'outFeature'; } @@ -666,9 +667,9 @@ var fixmystreet = fixmystreet || {}; var href = $('a', this).attr('href'); var id = parseInt(href.replace(/^.*[/]([0-9]+)$/, '$1')); clearTimeout(timeout); - markers_highlight(id); + fixmystreet.maps.markers_highlight(id); }).on('mouseleave', '.item-list--reports__item', function(){ - timeout = setTimeout(markers_highlight, 50); + timeout = setTimeout(fixmystreet.maps.markers_highlight, 50); }); })(); }); @@ -888,6 +889,19 @@ OpenLayers.Control.Click = OpenLayers.Class(OpenLayers.Control, { } }); +/* Drag handler that allows individual features to disable dragging */ +OpenLayers.Control.DragFeatureFMS = OpenLayers.Class(OpenLayers.Control.DragFeature, { + CLASS_NAME: "OpenLayers.Control.DragFeatureFMS", + + overFeature: function(feature) { + if (feature.attributes.draggable) { + return OpenLayers.Control.DragFeature.prototype.overFeature.call(this, feature); + } else { + return false; + } + } +}) + OpenLayers.Renderer.SVGBig = OpenLayers.Class(OpenLayers.Renderer.SVG, { MAX_PIXEL: 15E7, CLASS_NAME: "OpenLayers.Renderer.SVGBig" diff --git a/web/js/map-google.js b/web/js/map-google.js index 4c3f6188e..be2df8502 100644 --- a/web/js/map-google.js +++ b/web/js/map-google.js @@ -56,6 +56,9 @@ fixmystreet.maps = {}; google.maps.event.trigger(fixmystreet.map, 'idle'); }; + // This is a noop on Google Maps right now. + fixmystreet.maps.markers_highlight = function() {}; + function PaddingControl(div) { div.style.padding = '40px'; } -- cgit v1.2.3 From ec6389940afce877a0bc7771d11a27ee7183f96a Mon Sep 17 00:00:00 2001 From: Dave Arter Date: Tue, 22 Nov 2016 13:04:25 +0000 Subject: Make it clearer that report is closed when marked as duplicate - Record state change when leaving update and marking as duplicate - Change save button wording to match problem state when inspecting - Make it clearer that updates marking a report as duplicate actually close the report --- perllib/FixMyStreet/App/Controller/Report.pm | 7 +++++ t/app/controller/report_display.t | 2 +- t/app/controller/report_inspect.t | 46 +++++++++++++++++++++++----- t/app/controller/report_updates.t | 2 ++ templates/web/base/report/_inspect.html | 2 +- templates/web/base/report/updates.html | 2 +- web/cobrands/fixmystreet/fixmystreet.js | 13 +++++++- 7 files changed, 63 insertions(+), 11 deletions(-) diff --git a/perllib/FixMyStreet/App/Controller/Report.pm b/perllib/FixMyStreet/App/Controller/Report.pm index 154f0ac94..f7ccddc70 100644 --- a/perllib/FixMyStreet/App/Controller/Report.pm +++ b/perllib/FixMyStreet/App/Controller/Report.pm @@ -314,6 +314,7 @@ sub inspect : Private { my $valid = 1; my $update_text; my $reputation_change = 0; + my %update_params = (); if ($permissions->{report_inspect}) { foreach (qw/detailed_information traffic_information duplicate_of/) { @@ -341,6 +342,11 @@ sub inspect : Private { if ( $problem->state eq 'hidden' ) { $problem->get_photoset->delete_cached; } + if ( $problem->state eq 'duplicate' && $old_state ne 'duplicate' ) { + # If the report is being closed as duplicate, make sure the + # update records this. + $update_params{problem_state} = "duplicate"; + } if ( $problem->state ne 'duplicate' ) { $problem->unset_extra_metadata('duplicate_of'); } @@ -388,6 +394,7 @@ sub inspect : Private { state => 'confirmed', mark_fixed => 0, anonymous => 0, + %update_params, } ); } # This problem might no longer be visible on the current cobrand, diff --git a/t/app/controller/report_display.t b/t/app/controller/report_display.t index 9817a640d..fad8b2bbb 100644 --- a/t/app/controller/report_display.t +++ b/t/app/controller/report_display.t @@ -117,7 +117,7 @@ subtest "duplicate reports are signposted correctly" => sub { my $report2_id = $report2->id; ok $mech->get("/report/$report2_id"), "get '/report/$report2_id'"; - $mech->content_contains('This report is a duplicate.'); + $mech->content_contains('This report is a duplicate'); $mech->content_contains($report->title); $mech->log_out_ok; diff --git a/t/app/controller/report_inspect.t b/t/app/controller/report_inspect.t index b4d6636b9..56e6e957f 100644 --- a/t/app/controller/report_inspect.t +++ b/t/app/controller/report_inspect.t @@ -21,12 +21,14 @@ my $wodc = $mech->create_body_ok(2420, 'West Oxfordshire District Council', id = $mech->create_contact_ok( body_id => $wodc->id, category => 'Horses', email => 'horses@example.net' ); -my ($report) = $mech->create_problems_for_body(1, $oxon->id, 'Test', { +my ($report, $report2) = $mech->create_problems_for_body(2, $oxon->id, 'Test', { category => 'Cows', cobrand => 'fixmystreet', areas => ',2237,2420', whensent => \'current_timestamp', latitude => 51.847693, longitude => -1.355908, }); my $report_id = $report->id; +my $report2_id = $report2->id; + my $user = $mech->log_in_ok('test@example.com'); $user->update( { from_body => $oxon } ); @@ -94,11 +96,6 @@ FixMyStreet::override_config { }; subtest "test duplicate reports are shown" => sub { - my ($report2) = $mech->create_problems_for_body(1, $oxon->id, 'The other report is a duplicate of this one', { - category => $report->category, cobrand => 'fixmystreet', areas => ',2237,2420', - whensent => \'current_timestamp', - latitude => 51.847694, longitude => -1.355909, - }); my $old_state = $report->state; $report->set_extra_metadata('duplicate_of' => $report2->id); $report->state('duplicate'); @@ -107,7 +104,6 @@ FixMyStreet::override_config { $mech->get_ok("/report/$report_id"); $mech->content_contains($report2->title); - my $report2_id = $report2->id; $mech->get_ok("/report/$report2_id"); $mech->content_contains($report->title); @@ -116,6 +112,42 @@ FixMyStreet::override_config { $report->update; }; + subtest "marking a report as a duplicate with update correctly sets update status" => sub { + my $old_state = $report->state; + $report->comments->delete_all; + + $mech->get_ok("/report/$report_id"); + $mech->submit_form_ok({ button => 'save', with_fields => { state => 'Duplicate', duplicate_of => $report2->id, public_update => "This is a duplicate.", save_inspected => "1" } }); + $report->discard_changes; + + is $report->state, 'duplicate', 'report marked as duplicate'; + is $report->comments->search({ problem_state => 'duplicate' })->count, 1, 'update marking report as duplicate was left'; + + $report->update({ state => $old_state }); + }; + + subtest "marking a report as a duplicate doesn't clobber user-provided update" => sub { + my $old_state = $report->state; + $report->comments->delete_all; + + $mech->get_ok("/report/$report_id"); + my $update_text = "This text was entered as an update by the user."; + $mech->submit_form_ok({ button => 'save', with_fields => { + state => 'Duplicate', + duplicate_of => $report2->id, + public_update => $update_text, + save_inspected => "1", + }}); + $report->discard_changes; + + is $report->state, 'duplicate', 'report marked as duplicate'; + is $report->comments->search({ problem_state => 'duplicate' })->count, 1, 'update marked report as duplicate'; + $mech->content_contains($update_text); + $mech->content_lacks("Thank you for your report. This problem has already been reported."); + + $report->update({ state => $old_state }); + }; + foreach my $test ( { type => 'report_edit_priority', priority => 1 }, { type => 'report_edit_category', category => 1 }, diff --git a/t/app/controller/report_updates.t b/t/app/controller/report_updates.t index e1cd0da71..5a88097fa 100644 --- a/t/app/controller/report_updates.t +++ b/t/app/controller/report_updates.t @@ -685,6 +685,8 @@ for my $test ( my $meta_state = $test->{meta} || $test->{fields}->{state}; if ( $test->{reopened} ) { like $update_meta->[0], qr/reopened$/, 'update meta says reopened'; + } elsif ( $test->{state} eq 'duplicate' ) { + like $update_meta->[0], qr/closed as $meta_state$/, 'update meta includes state change'; } else { like $update_meta->[0], qr/marked as $meta_state$/, 'update meta includes state change'; } diff --git a/templates/web/base/report/_inspect.html b/templates/web/base/report/_inspect.html index 6f49f3ecb..ccaa756c5 100644 --- a/templates/web/base/report/_inspect.html +++ b/templates/web/base/report/_inspect.html @@ -158,7 +158,7 @@

        [% loc('Cancel') %] - +

        diff --git a/templates/web/base/report/updates.html b/templates/web/base/report/updates.html index fc2ac6c78..ff48ecbca 100644 --- a/templates/web/base/report/updates.html +++ b/templates/web/base/report/updates.html @@ -54,7 +54,7 @@ [%- ELSIF state == 'not responsible' %] [%- update_state = loc( "marked as not the council's responsibility" ) %] [%- ELSIF state == 'duplicate' %] - [%- update_state = loc( 'marked as a duplicate report' ) %] + [%- update_state = loc( 'closed as a duplicate report' ) %] [%- ELSIF state == 'internal referral' %] [%- update_state = loc( 'marked as an internal referral' ) %] [%- END %] diff --git a/web/cobrands/fixmystreet/fixmystreet.js b/web/cobrands/fixmystreet/fixmystreet.js index 2a5c85872..cd3b127d6 100644 --- a/web/cobrands/fixmystreet/fixmystreet.js +++ b/web/cobrands/fixmystreet/fixmystreet.js @@ -390,7 +390,6 @@ $.extend(fixmystreet.set_up, { }); }, - manage_duplicates: function() { // Deal with changes to report state by inspector/other staff, specifically // displaying nearby reports if it's changed to 'duplicate'. @@ -657,6 +656,18 @@ $.extend(fixmystreet.set_up, { $("form#report_inspect_form " + selector).removeClass("hidden"); }); + // The inspect form submit button can change depending on the selected state + $("#report_inspect_form [name=state]").change(function(){ + var state = $(this).val(); + var $submit = $("#report_inspect_form input[type=submit]"); + var value = $submit.attr('data-value-'+state); + if (value !== undefined) { + $submit.val(value); + } else { + $submit.val($submit.data('valueOriginal')); + } + }).change(); + $('.js-toggle-public-update').each(function() { var $checkbox = $(this); var toggle_public_update = function() { -- cgit v1.2.3