diff options
| -rw-r--r-- | perllib/FixMyStreet/App/Controller/Report/New.pm | 12 | ||||
| -rw-r--r-- | perllib/FixMyStreet/App/Controller/Tokens.pm | 4 | ||||
| -rw-r--r-- | perllib/FixMyStreet/Cobrand/Harrogate.pm | 6 | ||||
| -rw-r--r-- | perllib/FixMyStreet/Cobrand/Zurich.pm | 59 | ||||
| -rw-r--r-- | perllib/FixMyStreet/DB/Result/Contact.pm | 8 | ||||
| -rw-r--r-- | perllib/FixMyStreet/DB/Result/Problem.pm | 3 | ||||
| -rw-r--r-- | perllib/FixMyStreet/Roles/Extra.pm | 193 | ||||
| -rw-r--r-- | perllib/FixMyStreet/SendReport/Open311.pm | 18 | ||||
| -rw-r--r-- | perllib/Open311.pm | 20 | ||||
| -rw-r--r-- | perllib/Open311/PopulateServiceList.pm | 6 | ||||
| -rw-r--r-- | t/app/controller/report_new.t | 4 | ||||
| -rw-r--r-- | t/app/controller/report_new_open311.t | 2 | ||||
| -rw-r--r-- | t/app/controller/token.t | 37 | ||||
| -rw-r--r-- | t/app/model/extra.t | 111 | ||||
| -rw-r--r-- | t/cobrand/bromley.t | 20 | ||||
| -rw-r--r-- | t/cobrand/zurich.t | 9 | ||||
| -rw-r--r-- | t/open311/populate-service-list.t | 21 |
17 files changed, 452 insertions, 81 deletions
diff --git a/perllib/FixMyStreet/App/Controller/Report/New.pm b/perllib/FixMyStreet/App/Controller/Report/New.pm index b5a6745b2..b4daf0f55 100644 --- a/perllib/FixMyStreet/App/Controller/Report/New.pm +++ b/perllib/FixMyStreet/App/Controller/Report/New.pm @@ -636,8 +636,9 @@ sub setup_categories_and_bodies : Private { unless ( $seen{$contact->category} ) { push @category_options, $contact->category; - $category_extras{ $contact->category } = $contact->extra - if $contact->extra; + my $metas = $contact->get_extra_fields; + $category_extras{ $contact->category } = $metas + if scalar @$metas; $non_public_categories{ $contact->category } = 1 if $contact->non_public; } @@ -870,8 +871,9 @@ sub process_report : Private { if $body_string && @{ $c->stash->{missing_details_bodies} }; $report->bodies_str($body_string); - my @extra = (); - my $metas = $contacts[0]->extra; + my @extra; + # NB: we are only checking extras for the *first* retrieved contact. + my $metas = $contacts[0]->get_extra_fields(); foreach my $field ( @$metas ) { if ( lc( $field->{required} ) eq 'true' ) { @@ -894,7 +896,7 @@ sub process_report : Private { if ( @extra ) { $c->stash->{report_meta} = { map { $_->{name} => $_ } @extra }; - $report->extra( \@extra ); + $report->set_extra_fields( @extra ); } } elsif ( @{ $c->stash->{bodies_to_list} } ) { diff --git a/perllib/FixMyStreet/App/Controller/Tokens.pm b/perllib/FixMyStreet/App/Controller/Tokens.pm index c8a7531d6..4c0afdeda 100644 --- a/perllib/FixMyStreet/App/Controller/Tokens.pm +++ b/perllib/FixMyStreet/App/Controller/Tokens.pm @@ -66,10 +66,8 @@ sub confirm_problem : Path('/P') { # For Zurich, email confirmation simply sets a flag, it does not change the # problem state, log in, or anything else if ($c->cobrand->moniker eq 'zurich') { - my $extra = { %{ $problem->extra || {} } }; - $extra->{email_confirmed} = 1; + $problem->set_extra_metadata( email_confirmed => 1 ); $problem->update( { - extra => $extra, confirmed => \'ms_current_timestamp()', } ); diff --git a/perllib/FixMyStreet/Cobrand/Harrogate.pm b/perllib/FixMyStreet/Cobrand/Harrogate.pm index 4dc041c89..afda52385 100644 --- a/perllib/FixMyStreet/Cobrand/Harrogate.pm +++ b/perllib/FixMyStreet/Cobrand/Harrogate.pm @@ -112,7 +112,9 @@ sub temp_update_contacts { } $contact->update({ - extra => [ { %default, %$field } ], + # XXX: we're just setting extra with the expected layout, + # this could be encapsulated more nicely + extra => { _fields => [ { %default, %$field } ] }, confirmed => 1, deleted => 0, editor => 'automated script', @@ -223,7 +225,7 @@ sub process_additional_metadata_for_email { my ($self, $problem, $h) = @_; my $additional = ''; - if (my $extra = $problem->extra) { + if (my $extra = $problem->get_extra_fields) { $additional = join "\n\n", map { if ($_->{name} eq 'INFO_TEXT') { (); diff --git a/perllib/FixMyStreet/Cobrand/Zurich.pm b/perllib/FixMyStreet/Cobrand/Zurich.pm index 074c21edb..8d7424328 100644 --- a/perllib/FixMyStreet/Cobrand/Zurich.pm +++ b/perllib/FixMyStreet/Cobrand/Zurich.pm @@ -4,6 +4,7 @@ use base 'FixMyStreet::Cobrand::Default'; use DateTime; use POSIX qw(strcoll); use RABX; +use Scalar::Util 'blessed'; use strict; use warnings; @@ -158,7 +159,8 @@ sub updates_as_hashref { $hashref->{update_pp} = $self->prettify_dt( $problem->lastupdate ); if ( $problem->state eq 'fixed - council' ) { - $hashref->{details} = FixMyStreet::App::View::Web->add_links( $ctx, $problem->extra ? $problem->extra->{public_response} : '' ); + $hashref->{details} = FixMyStreet::App::View::Web->add_links( + $ctx, $problem->get_extra_metadata('public_response') || '' ); } elsif ( $problem->state eq 'closed' ) { $hashref->{details} = sprintf( _('Assigned to %s'), $problem->body($ctx)->name ); } @@ -169,13 +171,16 @@ sub updates_as_hashref { sub allow_photo_display { my ( $self, $r ) = @_; - if (ref($r) ne 'HASH') { - return $r->extra && $r->extra->{publish_photo}; + if (blessed $r) { + return $r->get_extra_metadata( 'publish_photo' ); } + + # additional munging in case $r isn't an object, TODO see if we can remove this my $extra = $r->{extra}; utf8::encode($extra) if utf8::is_utf8($extra); my $h = new IO::String($extra); $extra = RABX::wire_rd($h); + return unless ref $extra eq 'HASH'; return $extra->{publish_photo}; } @@ -264,10 +269,9 @@ sub get_or_check_overdue { my ($self, $problem) = @_; # use the cached version is it exists (e.g. when called from template) - my $extra = $problem->extra; - if (exists $extra->{closed_overdue} and defined $extra->{closed_overdue}) { - return $extra->{closed_overdue} - } + my $overdue = $problem->get_extra_metadata('closed_overdue'); + return $overdue if defined $overdue; + return $self->overdue($problem); } @@ -466,12 +470,18 @@ sub admin_report_edit { # Problem updates upon submission if ( ($type eq 'super' || $type eq 'dm') && $c->req->param('submit') ) { - # Predefine the hash so it's there for lookups - my $extra = $problem->extra || {}; - $extra->{publish_photo} = $c->req->params->{publish_photo} || 0; - $extra->{third_personal} = $c->req->params->{third_personal} || 0; + $problem->set_extra_metadata('publish_photo' => $c->req->params->{publish_photo} || 0 ); + $problem->set_extra_metadata('third_personal' => $c->req->params->{third_personal} || 0 ); + # Make sure we have a copy of the original detail field - $extra->{original_detail} = $problem->detail if !$extra->{original_detail} && $c->req->params->{detail} && $problem->detail ne $c->req->params->{detail}; + if (my $new_detail = $c->req->params->{detail}) { + my $old_detail = $problem->detail; + if (! $problem->get_extra_metadata('original_detail') + && ($old_detail ne $new_detail)) + { + $problem->set_extra_metadata( original_detail => $old_detail ); + } + } # Some changes will be accompanied by an internal note, which if needed # should be stored in this variable. @@ -487,20 +497,20 @@ sub admin_report_edit { $problem->external_body( undef ); $problem->bodies_str( $cat->body_id ); $problem->whensent( undef ); - $extra->{changed_category} = 1; + $problem->set_extra_metadata(changed_category => 1); $internal_note_text = "Weitergeleitet von $old_cat an $new_cat"; $redirect = 1 if $cat->body_id ne $body->id; } elsif ( my $subdiv = $c->req->params->{body_subdivision} ) { - $extra->{moderated_overdue} //= $self->overdue( $problem ); + $problem->set_extra_metadata_if_undefined( moderated_overdue => $self->overdue( $problem ) ); $self->set_problem_state($c, $problem, 'in progress'); $problem->external_body( undef ); $problem->bodies_str( $subdiv ); $problem->whensent( undef ); $redirect = 1; } elsif ( my $external = $c->req->params->{body_external} ) { - $extra->{moderated_overdue} //= $self->overdue( $problem ); + $problem->set_extra_metadata_if_undefined( moderated_overdue => $self->overdue( $problem ) ); $self->set_problem_state($c, $problem, 'closed'); - $extra->{closed_overdue} //= $self->overdue( $problem ); + $problem->set_extra_metadata_if_undefined( closed_overdue => $self->overdue( $problem ) ); $problem->external_body( $external ); $problem->whensent( undef ); _admin_send_email( $c, 'problem-external.txt', $problem ); @@ -510,13 +520,13 @@ sub admin_report_edit { if ($problem->state eq 'unconfirmed' and $state ne 'unconfirmed') { # only set this for the first state change - $extra->{moderated_overdue} //= $self->overdue( $problem ); + $problem->set_extra_metadata_if_undefined( moderated_overdue => $self->overdue( $problem ) ); } $self->set_problem_state($c, $problem, $state); if ($self->problem_is_closed($problem)) { - $extra->{closed_overdue} //= $self->overdue( $problem ); + $problem->set_extra_metadata_if_undefined( closed_overdue => $self->overdue( $problem ) ); } if ( $state eq 'hidden' && $c->req->params->{send_rejected_email} ) { _admin_send_email( $c, 'problem-rejected.txt', $problem ); @@ -524,7 +534,6 @@ sub admin_report_edit { } } - $problem->extra( $extra ); $problem->title( $c->req->param('title') ); $problem->detail( $c->req->param('detail') ); $problem->latitude( $c->req->param('latitude') ); @@ -532,12 +541,10 @@ sub admin_report_edit { # Final, public, Update from DM if (my $update = $c->req->param('status_update')) { - $extra->{public_response} = $update; - $problem->extra( $extra ); + $problem->set_extra_metadata(public_response => $update); if ($c->req->params->{publish_response}) { $self->set_problem_state($c, $problem, 'fixed - council'); - $extra->{closed_overdue} = $self->overdue( $problem ); - $problem->extra( { %$extra } ); + $problem->set_extra_metadata( closed_overdue => $self->overdue( $problem ) ); _admin_send_email( $c, 'problem-closed.txt', $problem ); } } @@ -619,9 +626,7 @@ sub admin_report_edit { # If they clicked the no more updates button, we're done. if ($c->req->param('no_more_updates')) { - my $extra = $problem->extra || {}; - $extra->{subdiv_overdue} = $self->overdue( $problem ); - $problem->extra( $extra ); + $problem->set_extra_metadata( subdiv_overdue => $self->overdue( $problem ) ); $problem->bodies_str( $body->parent->id ); $problem->whensent( undef ); $self->set_problem_state($c, $problem, 'planned'); @@ -645,7 +650,7 @@ sub admin_report_edit { sub _admin_send_email { my ( $c, $template, $problem ) = @_; - return unless $problem->extra && $problem->extra->{email_confirmed}; + return unless $problem->get_extra_metadata('email_confirmed'); my $to = $problem->name ? [ $problem->user->email, $problem->name ] diff --git a/perllib/FixMyStreet/DB/Result/Contact.pm b/perllib/FixMyStreet/DB/Result/Contact.pm index eca028c9b..2fbb0716d 100644 --- a/perllib/FixMyStreet/DB/Result/Contact.pm +++ b/perllib/FixMyStreet/DB/Result/Contact.pm @@ -63,4 +63,12 @@ __PACKAGE__->belongs_to( __PACKAGE__->load_components("+FixMyStreet::DB::RABXColumn"); __PACKAGE__->rabx_column('extra'); +use Moose; +use namespace::clean -except => [ 'meta' ]; + +with 'FixMyStreet::Roles::Extra'; + +# we need the inline_constructor bit as we don't inherit from Moose +__PACKAGE__->meta->make_immutable( inline_constructor => 0 ); + 1; diff --git a/perllib/FixMyStreet/DB/Result/Problem.pm b/perllib/FixMyStreet/DB/Result/Problem.pm index 3c620ba84..93dc16811 100644 --- a/perllib/FixMyStreet/DB/Result/Problem.pm +++ b/perllib/FixMyStreet/DB/Result/Problem.pm @@ -158,7 +158,8 @@ use Moose; use namespace::clean -except => [ 'meta' ]; use Utils; -with 'FixMyStreet::Roles::Abuser'; +with 'FixMyStreet::Roles::Abuser', + 'FixMyStreet::Roles::Extra'; =head2 diff --git a/perllib/FixMyStreet/Roles/Extra.pm b/perllib/FixMyStreet/Roles/Extra.pm new file mode 100644 index 000000000..f815a3e9a --- /dev/null +++ b/perllib/FixMyStreet/Roles/Extra.pm @@ -0,0 +1,193 @@ +package FixMyStreet::Roles::Extra; +use Moose::Role; + +=head1 NAME + +FixMyStreet::Roles::Extra - role for accessing {extra} field + +=head1 SYNOPSIS + +This is to applied to a DB class like Problem or Contacts that has a rich {extra} field: + + use Moose; + with 'FixMyStreet::Roles::Extra'; + +(NB: there is actually a little more boilerplate, because DBIC doesn't actually +inherit from Moose, see ::Problem for an example.) + +Then: + + $contact->set_extra_fields( + { name => 'pothole_size', ... }, + { name => 'pothole_shape, ... } ); + my $metas = $contact->get_extra_fields(); + +And + + # e.g. for sites like Zurich (but handled gracefully otherwise) + $problem->set_extra_metadata( overdue => 1 ); + if ($problem->get_extra_metadata( 'overdue')) { ... } + +=head1 METHODS + +=head2 set_extra_metadata + + $problem->set_extra_metadata( overdue => 1); + +=cut + +sub set_extra_metadata { + my ($self, $key, $value) = @_; + my $extra = $self->get_extra(); + + $self->extra({ %$extra, $key => $value }); +}; + +=head2 set_extra_metadata_if_undefined + + $problem->set_extra_metadata_if_undefined( overdue => 1); + # as above, but won't set if, for example 'overdue' is already set to 0 + +=cut + +sub set_extra_metadata_if_undefined { + my ($self, $key, $value) = @_; + my $extra = $self->get_extra(); + + return if defined $extra->{$key}; + $self->extra({ %$extra, $key => $value }); +}; + +=head2 unset_extra_metadata + + $contact->unset_extra_metadata('photo_required'); + +=cut + +sub unset_extra_metadata { + my ($self, $key) = @_; + my $extra = $self->get_extra(); + + return 1 unless exists $extra->{$key}; + delete $extra->{$key}; + $self->extra($extra); +}; + +=head2 get_extra_metadata + + my $overdue = $problem->get_extra_metadata('overdue'); + +=cut + +sub get_extra_metadata { + my ($self, $key) = @_; + my $extra = $self->get_extra(); + + return $extra->{$key}; +}; + +=head2 get_extra_metadata_as_hashref + + my $hashref = $contact->get_extra_metadata_as_hashref(); + +=cut + +my $META_FIELD = '_fields'; + +sub get_extra_metadata_as_hashref { + my ($self) = @_; + my $extra = $self->get_extra(); + + my %extra = %$extra; + delete $extra{$META_FIELD}; + return \%extra; +} + +=head2 get_extra_fields + + my $metas = $problem->get_extra_fields(); + +=cut + +sub get_extra_fields { + my ($self) = @_; + my $extra = $self->get_extra(); + + return $extra->{$META_FIELD} ||= do { + my $metas = []; + $self->extra({ %$extra, $META_FIELD => $metas }); + $metas; + }; +} + +=head2 set_extra_fields + + $problem->set_extra_fields( { ... }, { ... } ); + +=cut + +sub set_extra_fields { + my ($self, @fields) = @_; + my $extra = $self->get_extra(); + + $self->extra({ %$extra, $META_FIELD => \@fields }); +} + +=head2 push_extra_fields + + $problem->push_extra_fields( { ... } ); + +like set_extra_fields, but pushes the new fields onto the end of the existing list. + +=cut + +sub push_extra_fields { + my ($self, @fields) = @_; + my $extra = $self->get_extra(); + + my $existing = $self->get_extra_fields; + + $self->extra({ %$extra, $META_FIELD => [ @$existing, @fields ] }); +} + +=head1 HELPER METHODS + +For internal use mostly. + +=head2 dirty_extra + +Set the extra field as dirty. (e.g. signalling that the DB object should be +updated). + +=cut + +sub dirty_extra { + my $self = shift; + $self->make_column_dirty('extra'); + return 1; +} + +=head2 get_extra + +Get the extra data. If this is not set, then returns a {} + +=cut + +sub get_extra { + my ($self) = @_; + my $extra = $self->extra or do { + my $extra = {}; + $self->extra({}); + return $extra; + }; + + if (ref $extra eq 'ARRAY') { + # upgrade layout transparently + $extra = { $META_FIELD => $extra }; + $self->extra($extra); + } + + return $extra; +} + +1; diff --git a/perllib/FixMyStreet/SendReport/Open311.pm b/perllib/FixMyStreet/SendReport/Open311.pm index c064eeef5..d65a8ddb1 100644 --- a/perllib/FixMyStreet/SendReport/Open311.pm +++ b/perllib/FixMyStreet/SendReport/Open311.pm @@ -37,7 +37,7 @@ sub send { $revert = 1; - my $extra = $row->extra; + my $extra = $row->get_extra_fields(); if ( $row->used_map || ( !$row->used_map && !$row->postcode ) ) { push @$extra, { name => 'northing', value => $h->{northing} }; push @$extra, { name => 'easting', value => $h->{easting} }; @@ -49,33 +49,31 @@ sub send { push @$extra, { name => 'email_alerts_requested', value => 'FALSE' }; # always false as can never request them push @$extra, { name => 'requested_datetime', value => DateTime::Format::W3CDTF->format_datetime($row->confirmed->set_nanosecond(0)) }; push @$extra, { name => 'email', value => $row->user->email }; - $row->extra( $extra ); - - $always_send_latlong = 0; - $send_notpinpointed = 1; - $use_service_as_deviceid = 0; - # make sure we have last_name attribute present in row's extra, so # it is passed correctly to Bromley as attribute[] if ( $row->cobrand ne 'bromley' ) { my ( $firstname, $lastname ) = ( $row->name =~ /(\w+)\.?\s+(.+)/ ); push @$extra, { name => 'last_name', value => $lastname }; } + $row->set_extra_fields( @$extra ); + $always_send_latlong = 0; + $send_notpinpointed = 1; + $use_service_as_deviceid = 0; $extended_desc = 0; } # extra Oxfordshire fields: send nearest street, postcode, northing and easting, and the FMS id if ( $row->bodies_str =~ /\b(?:$COUNCIL_ID_OXFORDSHIRE|$COUNCIL_ID_WARWICKSHIRE)\b/ ) { - my $extra = $row->extra; + my $extra = $row->get_extra_fields; push @$extra, { name => 'external_id', value => $row->id }; push @$extra, { name => 'closest_address', value => $h->{closest_address} } if $h->{closest_address}; if ( $row->used_map || ( !$row->used_map && !$row->postcode ) ) { push @$extra, { name => 'northing', value => $h->{northing} }; push @$extra, { name => 'easting', value => $h->{easting} }; } - $row->extra( $extra ); + $row->set_extra_fields( @$extra ); if ($row->bodies_str =~ /$COUNCIL_ID_OXFORDSHIRE/) { $extended_desc = 'oxfordshire'; @@ -131,7 +129,7 @@ sub send { if ($row->cobrand eq 'fixmybarangay') { # FixMyBarangay endpoints expect external_id as an attribute, as do Oxfordshire - $row->extra( [ { 'name' => 'external_id', 'value' => $row->id } ] ); + $row->set_extra_fields( { 'name' => 'external_id', 'value' => $row->id } ); $revert = 1; } diff --git a/perllib/Open311.pm b/perllib/Open311.pm index c02618725..58ae96bc2 100644 --- a/perllib/Open311.pm +++ b/perllib/Open311.pm @@ -150,19 +150,15 @@ sub _populate_service_request_params { $params->{deviceid} = $problem->service; } - if ( $problem->extra ) { - my $extras = $problem->extra; - - for my $attr ( @$extras ) { - my $attr_name = $attr->{name}; - if ( $attr_name eq 'first_name' || $attr_name eq 'last_name' ) { - $params->{$attr_name} = $attr->{value} if $attr->{value}; - next if $attr_name eq 'first_name'; - } - $attr_name =~ s/fms_extra_//; - my $name = sprintf( 'attribute[%s]', $attr_name ); - $params->{ $name } = $attr->{value}; + for my $attr ( @{$problem->get_extra_fields} ) { + my $attr_name = $attr->{name}; + if ( $attr_name eq 'first_name' || $attr_name eq 'last_name' ) { + $params->{$attr_name} = $attr->{value} if $attr->{value}; + next if $attr_name eq 'first_name'; } + $attr_name =~ s/fms_extra_//; + my $name = sprintf( 'attribute[%s]', $attr_name ); + $params->{ $name } = $attr->{value}; } return $params; diff --git a/perllib/Open311/PopulateServiceList.pm b/perllib/Open311/PopulateServiceList.pm index 0f6e32893..e4f0b8357 100644 --- a/perllib/Open311/PopulateServiceList.pm +++ b/perllib/Open311/PopulateServiceList.pm @@ -265,11 +265,7 @@ sub _add_meta_to_contact { @meta = grep { ! $ignore{ $_->{ code } } } @meta; } - if ( @meta ) { - $contact->extra( \@meta ); - } else { - $contact->extra( undef ); - } + $contact->set_extra_fields(@meta); $contact->update; } diff --git a/t/app/controller/report_new.t b/t/app/controller/report_new.t index 25fa01a1f..3e1446068 100644 --- a/t/app/controller/report_new.t +++ b/t/app/controller/report_new.t @@ -999,7 +999,7 @@ for my $test ( host => 'www.fixmystreet.com', postcode => 'EH99 1SP', fms_extra_title => '', - extra => undef, + extra => [], user_title => undef, }, { @@ -1127,7 +1127,7 @@ for my $test ( my $report = $user->problems->first; ok $report, "Found the report"; - my $extras = $report->extra; + my $extras = $report->get_extra_fields; is $user->title, $test->{'user_title'}, 'user title correct'; is_deeply $extras, $test->{extra}, 'extra contains correct values'; diff --git a/t/app/controller/report_new_open311.t b/t/app/controller/report_new_open311.t index 15ccef493..53b84b92d 100644 --- a/t/app/controller/report_new_open311.t +++ b/t/app/controller/report_new_open311.t @@ -158,7 +158,7 @@ foreach my $test ( my $prob = $user->problems->first; ok $prob, 'problem created'; - is_deeply $prob->extra, $test->{extra}, 'extra open311 data added to problem'; + is_deeply $prob->get_extra_fields, $test->{extra}, 'extra open311 data added to problem'; $user->problems->delete; $user->delete; diff --git a/t/app/controller/token.t b/t/app/controller/token.t new file mode 100644 index 000000000..9ca8b905d --- /dev/null +++ b/t/app/controller/token.t @@ -0,0 +1,37 @@ +use strict; +use warnings; +use Test::More; +use utf8; + +use FixMyStreet::TestMech; +use FixMyStreet::App; + +my $user = FixMyStreet::App->model('DB::User')->find_or_create({ + name => 'Bob', email => 'bob@example.com', + }); +my $mech = FixMyStreet::TestMech->new; + +subtest 'Zurich special case for C::Tokens->problem_confirm' => sub { + FixMyStreet::override_config { + ALLOWED_COBRANDS => ['zurich'], + }, sub { + my $c = FixMyStreet::App->new; + my $zurich = $mech->create_body_ok( 1, 'Zurich' ); + my ($report) = $mech->create_problems_for_body( + 1, $zurich->id, + { + state => 'unconfirmed', + confirmed => undef, + cobrand => 'zurich', + }); + + is $report->get_extra_metadata('email_confirmed'), undef, 'email_confirmed not yet set (sanity)'; + my $token = $c->model('DB::Token')->create({ scope => 'problem', data => $report->id }); + + $mech->get_ok('/P/' . $token->token); + $report->discard_changes; + is $report->get_extra_metadata('email_confirmed'), 1, 'email_confirmed set by Zurich special case'; + }; +}; + +done_testing; diff --git a/t/app/model/extra.t b/t/app/model/extra.t new file mode 100644 index 000000000..21c37336e --- /dev/null +++ b/t/app/model/extra.t @@ -0,0 +1,111 @@ +use strict; +use warnings; +use Test::More; +use utf8; + +use FixMyStreet::App; +use Data::Dumper; +use DateTime; + +my $c = FixMyStreet::App->new; + +my $db = FixMyStreet::App->model('DB')->schema; +$db->txn_begin; + +my $body = $db->resultset('Body')->create({ name => 'ExtraTestingBody' }); + +my $serial = 1; +sub get_test_contact { + my $extra = shift; + my $contact = $db->resultset('Contact')->create({ + category => "Testing ${serial}", + body => $body, + email => 'test@example.com', + confirmed => 1, + deleted => 0, + editor => 'test script', + note => 'test script', + whenedited => DateTime->now(), + $extra ? ( extra => $extra ) : (), + }); + $serial++; + return $contact; +} + +subtest 'Old list layout transparently upgraded' => sub { + + subtest 'layout' => sub { + my $contact = get_test_contact([]); + + is_deeply $contact->get_extra(), { _fields => [] }, 'transparently upgraded to a hash'; + }; + + subtest 'extra fields' => sub { + my $contact = get_test_contact([]); + + is_deeply $contact->get_extra_fields(), [], 'No extra fields'; + + my @fields = ( { a => 1 }, { b => 2 } ); + $contact->set_extra_fields(@fields); + is_deeply $contact->extra, { _fields => \@fields }, 'extra fields set...'; + $contact->update; + $contact->discard_changes; + is_deeply $contact->extra, { _fields => \@fields }, '...and retrieved'; + is_deeply $contact->get_extra_fields(), \@fields, 'extra fields returned'; + }; + + subtest 'metadata' => sub { + my $contact = get_test_contact([]); + is_deeply $contact->get_extra_metadata_as_hashref(), {}, 'No extra metadata'; + + $contact->set_extra_metadata('foo' => 'bar'); + is $contact->get_extra_metadata('foo'), 'bar', 'extra metadata set...'; + $contact->update; + $contact->discard_changes; + is $contact->get_extra_metadata('foo'), 'bar', '... and retrieved'; + is_deeply $contact->get_extra_metadata_as_hashref(), { foo => 'bar' }, 'No extra metadata'; + }; +}; + +subtest 'Default hash layout' => sub { + subtest 'layout' => sub { + my $contact = get_test_contact(); + + is_deeply $contact->get_extra(), {}, 'default layout is hash'; + }; + + subtest 'extra fields' => sub { + my $contact = get_test_contact(); + + is_deeply $contact->get_extra_fields(), [], 'No extra fields'; + + my @fields = ( { a => 1 }, { b => 2 } ); + $contact->set_extra_fields(@fields); + is_deeply $contact->get_extra_fields, \@fields, 'extra fields set...'; + $contact->update; + $contact->discard_changes; + is_deeply $contact->get_extra_fields(), \@fields, '... and returned'; + is_deeply $contact->extra, { _fields => \@fields }, '(sanity check layout)'; + }; + + subtest 'metadata' => sub { + my $contact = get_test_contact(); + is_deeply $contact->get_extra_metadata_as_hashref(), {}, 'No extra metadata'; + + $contact->set_extra_metadata('foo' => 'bar'); + is $contact->get_extra_metadata('foo'), 'bar', 'extra metadata set...'; + $contact->update; + $contact->discard_changes; + is $contact->get_extra_metadata( 'foo'), 'bar', '... and retrieved'; + is_deeply $contact->get_extra_metadata_as_hashref(), { foo => 'bar' }, 'No extra metadata'; + + $contact->unset_extra_metadata('foo'); + is $contact->get_extra_metadata('foo'), undef, 'extra metadata now unset'; + $contact->update; + $contact->discard_changes; + is $contact->get_extra_metadata('foo'), undef, '... after retrieval'; + }; +}; + +$db->txn_rollback; +done_testing(); diff --git a/t/cobrand/bromley.t b/t/cobrand/bromley.t index 6437ba3bd..fdded5606 100644 --- a/t/cobrand/bromley.t +++ b/t/cobrand/bromley.t @@ -8,6 +8,11 @@ my $mech = FixMyStreet::TestMech->new; # Create test data my $user = $mech->create_user_ok( 'bromley@example.com' ); my $body = $mech->create_body_ok( 2482, 'Bromley', id => 2482 ); +$mech->create_contact_ok( + body_id => $body->id, + category => 'Other', + email => 'LIGHT', +); my @reports = $mech->create_problems_for_body( 1, $body->id, 'Test', { cobrand => 'bromley', @@ -36,6 +41,21 @@ $mech->content_contains( 'marked as in progress' ); $mech->content_contains( 'marks it as unable to fix' ); $mech->content_contains( 'marked as no further action' ); +subtest 'testing special Open311 behaviour', sub { + $report->set_extra_fields(); + $report->update; + $body->update( { send_method => 'Open311', endpoint => 'http://bromley.endpoint.example.com', jurisdiction => 'FMS', api_key => 'test' } ); + FixMyStreet::override_config { + SEND_REPORTS_ON_STAGING => 1, + }, sub { + FixMyStreet::App->model('DB::Problem')->send_reports(); + }; + $report->discard_changes; + ok $report->whensent, 'Report marked as sent'; + is $report->send_method_used, 'Open311', 'Report sent via Open311'; + is $report->external_id, 248, 'Report has right external ID'; +}; + # Clean up $mech->delete_user($user); $mech->delete_problems_for_body( $body->id ); diff --git a/t/cobrand/zurich.t b/t/cobrand/zurich.t index 203bbc0f8..31aceab28 100644 --- a/t/cobrand/zurich.t +++ b/t/cobrand/zurich.t @@ -17,13 +17,16 @@ use JSON; # commonlib/bin/gettext-makemo FixMyStreet use FixMyStreet; +my $c = FixMyStreet::App->new(); +my $cobrand = FixMyStreet::Cobrand::Zurich->new({ c => $c }); +$c->stash->{cobrand} = $cobrand; # This is a helper method that will send the reports but with the config # correctly set - notably SEND_REPORTS_ON_STAGING needs to be true. sub send_reports_for_zurich { FixMyStreet::override_config { SEND_REPORTS_ON_STAGING => 1 }, sub { # Actually send the report - FixMyStreet::App->model('DB::Problem')->send_reports('zurich'); + $c->model('DB::Problem')->send_reports('zurich'); }; } sub reset_report_state { @@ -285,9 +288,9 @@ subtest "report_edit" => sub { is ( $report->extra->{closed_overdue}, 0, 'Marking hidden from scratch also set closed_overdue' ); is get_moderated_count(), 1; - is (FixMyStreet::Cobrand::Zurich->new->get_or_check_overdue($report), 0, 'sanity check'); + is ($cobrand->get_or_check_overdue($report), 0, 'sanity check'); $report->update({ created => $created->clone->subtract(days => 10) }); - is (FixMyStreet::Cobrand::Zurich->new->get_or_check_overdue($report), 0, 'overdue call not increased'); + is ($cobrand->get_or_check_overdue($report), 0, 'overdue call not increased'); reset_report_state($report, $created); }; diff --git a/t/open311/populate-service-list.t b/t/open311/populate-service-list.t index b343b206d..99663030a 100644 --- a/t/open311/populate-service-list.t +++ b/t/open311/populate-service-list.t @@ -254,14 +254,14 @@ subtest 'check meta data population' => sub { $contact->discard_changes; - is_deeply $contact->extra, $extra, 'meta data saved'; + is_deeply $contact->get_extra_fields, $extra, 'meta data saved'; }; for my $test ( { desc => 'check meta data added to existing contact', has_meta => 1, - orig_meta => undef, + orig_meta => [], end_meta => [ { variable => 'true', code => 'type', @@ -332,7 +332,7 @@ for my $test ( { desc => 'check meta data removed', has_meta => 0, - end_meta => undef, + end_meta => [], orig_meta => [ { variable => 'true', code => 'type', @@ -363,8 +363,8 @@ for my $test ( { desc => 'check empty meta data handled', has_meta => 1, - orig_meta => undef, - end_meta => undef, + orig_meta => [], + end_meta => [], meta_xml => '<?xml version="1.0" encoding="utf-8"?> <service_definition> <service_code>100</service_code> @@ -408,7 +408,8 @@ for my $test ( } ); - $contact->update( { extra => $test->{orig_meta} } ); + $contact->set_extra_fields(@{$test->{orig_meta}}); + $contact->update; my $o = Open311->new( jurisdiction => 'mysociety', @@ -427,7 +428,7 @@ for my $test ( $contact->discard_changes; - is_deeply $contact->extra, $test->{end_meta}, 'meta data saved'; + is_deeply $contact->get_extra_fields, $test->{end_meta}, 'meta data saved'; }; } @@ -530,7 +531,7 @@ subtest 'check attribute ordering' => sub { $contact->discard_changes; - is_deeply $contact->extra, $extra, 'meta data re-ordered correctly'; + is_deeply $contact->get_extra_fields, $extra, 'meta data re-ordered correctly'; }; subtest 'check bromely skip code' => sub { @@ -610,7 +611,7 @@ subtest 'check bromely skip code' => sub { $contact->discard_changes; - is_deeply $contact->extra, $extra, 'only non std bromley meta data saved'; + is_deeply $contact->get_extra_fields, $extra, 'only non std bromley meta data saved'; $processor->_current_body( $body ); $processor->_add_meta_to_contact( $contact ); @@ -650,7 +651,7 @@ subtest 'check bromely skip code' => sub { $contact->discard_changes; - is_deeply $contact->extra, $extra, 'all meta data saved for non bromley'; + is_deeply $contact->get_extra_fields, $extra, 'all meta data saved for non bromley'; }; sub get_standard_xml { |