From de9574686efd66f629c4cc861051f49c0a512605 Mon Sep 17 00:00:00 2001 From: Struan Donald Date: Wed, 12 Apr 2017 17:44:58 +0100 Subject: [PATCH 1/9] method to return a policy or policy set url for an MP --- classes/Member.php | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/classes/Member.php b/classes/Member.php index f1bdf48696..aa3607aded 100644 --- a/classes/Member.php +++ b/classes/Member.php @@ -236,6 +236,20 @@ public function offices($include_only = NULL, $ignore_committees = FALSE) { } + + public function getPolicyURL($args = array()) { + $base_url = $this->url(); + if (isset($args['policy_number'])) { + $url = $base_url . '/divisions?policy=' . $args['policy_number']; + } else if (isset($args['policy_set'])) { + $url = $base_url . '/votes?policy=' . $args['policy_set']; + } else { + $url = $base_url . '/votes'; + } + + return $url; + } + private function getOfficeObject($include_only, $ignore_committees, $row) { if (!$this->includeOffice($include_only, $row['to_date'])) { return null; From 96976c6b63fee14fb1fc140b693df6ce10414028 Mon Sep 17 00:00:00 2001 From: Struan Donald Date: Wed, 12 Apr 2017 17:51:44 +0100 Subject: [PATCH 2/9] optional hidden fields for the postcode form Used if we need to pass extra things like policy numbers along to the postcode page --- www/includes/easyparliament/page.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/www/includes/easyparliament/page.php b/www/includes/easyparliament/page.php index 73cbea9f4e..f3f19bb9c7 100644 --- a/www/includes/easyparliament/page.php +++ b/www/includes/easyparliament/page.php @@ -357,6 +357,11 @@ public function postcode_form() { echo '
'; $this->block_start(array('id'=>'mp', 'title'=>'Find out about your MP/MSPs/MLAs')); echo '
'; + if (count($hidden) > 0) { + foreach ($hidden as $field => $value) { + echo(''); + } + } if ($THEUSER->postcode_is_set()) { $FORGETURL = new URL('userchangepc'); $FORGETURL->insert(array('forget'=>'t')); From f85b3b69540a520aea2b41d062142d50719c4a3b Mon Sep 17 00:00:00 2001 From: Struan Donald Date: Wed, 12 Apr 2017 17:51:05 +0100 Subject: [PATCH 3/9] allow redirects from postcode page to policies or policy sets Used for linking to policy sets from outside TWFY, e.g. from social media. --- www/docs/postcode/index.php | 33 ++++++++++++++++++++++------ www/includes/easyparliament/page.php | 2 +- 2 files changed, 27 insertions(+), 8 deletions(-) diff --git a/www/docs/postcode/index.php b/www/docs/postcode/index.php index cbd00bbb4d..df5799ef2c 100644 --- a/www/docs/postcode/index.php +++ b/www/docs/postcode/index.php @@ -26,16 +26,19 @@ } $out = ''; $sidebars = array(); -if (isset($constituencies['SPE']) || isset($constituencies['SPC'])) { +$options = get_policy_array(); +// if there is a policy set or a policy number then assume we want the MP +// as those arguments only make sense for MPs +if (count($options) == 0 && (isset($constituencies['SPE']) || isset($constituencies['SPC']))) { $MEMBER = fetch_mp($pc, $constituencies); list($out, $sidebars) = pick_multiple($pc, $constituencies, 'SPE', 'MSP'); -} elseif (isset($constituencies['NIE'])) { +} elseif (count($options) == 0 && isset($constituencies['NIE'])) { $MEMBER = fetch_mp($pc, $constituencies); list($out, $sidebars) = pick_multiple($pc, $constituencies, 'NIE', 'MLA'); } else { # Just have an MP, redirect instantly to the canonical page $MEMBER = fetch_mp($pc, $constituencies, 1); - member_redirect($MEMBER); + member_redirect($MEMBER, $options); } $PAGE->page_start(); @@ -48,10 +51,11 @@ function postcode_error($error) { global $PAGE; + $hidden = get_policy_array(); $PAGE->page_start(); $PAGE->stripe_start(); $PAGE->error_message($error); - $PAGE->postcode_form(); + $PAGE->postcode_form($hidden); $PAGE->stripe_end(); $PAGE->page_end(); exit; @@ -64,7 +68,7 @@ function fetch_mp($pc, $constituencies, $house=null) { $args['house'] = $house; } try { - $MEMBER = new MEMBER($args); + $MEMBER = new \MySociety\TheyWorkForYou\Member($args); } catch (MySociety\TheyWorkForYou\MemberException $e){ postcode_error($e->getMessage()); } @@ -160,10 +164,25 @@ function pick_multiple($pc, $areas, $area_type, $rep_type) { return array($out, $sidebar); } -function member_redirect(&$MEMBER) { +function member_redirect(&$MEMBER, $options = array()) { if ($MEMBER->valid) { - $url = $MEMBER->url(); + if (count($options) > 0 ) { + $url = $MEMBER->getPolicyURL($options); + } else { + $url = $MEMBER->url(); + } header("Location: $url"); exit; } } + +function get_policy_array() { + $policy_options = array(); + if ($policy_set = get_http_var('policy_set')) { + $policy_options['policy_set'] = $policy_set; + } else if ($policy_number = get_http_var('policy_number')) { + $policy_options['policy_number'] = $policy_number; + } + + return $policy_options; +} diff --git a/www/includes/easyparliament/page.php b/www/includes/easyparliament/page.php index f3f19bb9c7..bce3d98b66 100644 --- a/www/includes/easyparliament/page.php +++ b/www/includes/easyparliament/page.php @@ -349,7 +349,7 @@ public function heading() { $this->heading_displayed = true; } - public function postcode_form() { + public function postcode_form($hidden = array()) { // Used on the mp (and yourmp) pages. // And the userchangepc page. global $THEUSER; From 07de47bb801179fc0795113a7a58874552e083f7 Mon Sep 17 00:00:00 2001 From: Struan Donald Date: Thu, 13 Apr 2017 10:58:15 +0100 Subject: [PATCH 4/9] optional title argument for postcode page Allow a different title on the postcode page for e.g. cases where we will be redirecting afterwards and we want to reassure the user that this is part of the process. --- www/includes/easyparliament/page.php | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/www/includes/easyparliament/page.php b/www/includes/easyparliament/page.php index bce3d98b66..cae89d808d 100644 --- a/www/includes/easyparliament/page.php +++ b/www/includes/easyparliament/page.php @@ -349,13 +349,17 @@ public function heading() { $this->heading_displayed = true; } - public function postcode_form($hidden = array()) { + public function postcode_form($hidden = array(), $title = '') { // Used on the mp (and yourmp) pages. // And the userchangepc page. global $THEUSER; + if ($title == '') { + $title = 'Find out about your MP/MSPs/MLAs'; + } + echo '
'; - $this->block_start(array('id'=>'mp', 'title'=>'Find out about your MP/MSPs/MLAs')); + $this->block_start(array('id'=>'mp', 'title'=>$title)); echo ''; if (count($hidden) > 0) { foreach ($hidden as $field => $value) { From fbcd7f365593643e21c8214e0195c96da8295b60 Mon Sep 17 00:00:00 2001 From: Struan Donald Date: Thu, 13 Apr 2017 11:00:04 +0100 Subject: [PATCH 5/9] refer to policy we are redirecting to in postcode form title If we are redirecting to a policy or policy set then change the title of the postcode form to mention the policy/set to reassure the user they are on the right track. --- www/docs/postcode/index.php | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/www/docs/postcode/index.php b/www/docs/postcode/index.php index df5799ef2c..cdc7927b12 100644 --- a/www/docs/postcode/index.php +++ b/www/docs/postcode/index.php @@ -52,10 +52,26 @@ function postcode_error($error) { global $PAGE; $hidden = get_policy_array(); + $title = ''; + if (count($hidden) > 0 ) { + $policies = new \MySociety\TheyWorkForYou\Policies(); + + if (isset($hidden['policy_number'])) { + $policies = $policies->getPolicies(); + if (isset($policies[$hidden['policy_number']])) { + $title = "Find out how your MP voted on " . $policies[$hidden['policy_number']] . "."; + } + } else if (isset($hidden['policy_set'])) { + $sets = $policies->getSetDescriptions(); + if (isset($sets[$hidden['policy_set']])) { + $title = "Find out how your MP voted on " . $sets[$hidden['policy_set']] . "."; + } + } + } $PAGE->page_start(); $PAGE->stripe_start(); $PAGE->error_message($error); - $PAGE->postcode_form($hidden); + $PAGE->postcode_form($hidden, $title); $PAGE->stripe_end(); $PAGE->page_end(); exit; From d30a7a10e9f023c1723608a0a940b40df013e29f Mon Sep 17 00:00:00 2001 From: Struan Donald Date: Thu, 13 Apr 2017 16:05:20 +0100 Subject: [PATCH 6/9] print postcode redirect if run under CLI for testing The test framework runs under CLI which means the headers aren't printed so we can't test redirects. To get round this check if we are running under the CLI and then print out the header. --- www/docs/postcode/index.php | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/www/docs/postcode/index.php b/www/docs/postcode/index.php index cdc7927b12..f65e5f2955 100644 --- a/www/docs/postcode/index.php +++ b/www/docs/postcode/index.php @@ -187,7 +187,14 @@ function member_redirect(&$MEMBER, $options = array()) { } else { $url = $MEMBER->url(); } - header("Location: $url"); + // awfulness so we can test this works becuase we use the CLI + // in our test framework for page requests and the CLI doesn't + // print out headers + if (php_sapi_name() === 'cli') { + print("Location: $url"); + } else { + header("Location: $url"); + } exit; } } From 63431d6ae237b3ccea4dbbefdb057b48daa1f304 Mon Sep 17 00:00:00 2001 From: Struan Donald Date: Thu, 13 Apr 2017 16:01:06 +0100 Subject: [PATCH 7/9] tests for the postcode page --- tests/PostcodePageTest.php | 76 ++++++++++ tests/_fixtures/postcodepage.xml | 241 +++++++++++++++++++++++++++++++ 2 files changed, 317 insertions(+) create mode 100644 tests/PostcodePageTest.php create mode 100644 tests/_fixtures/postcodepage.xml diff --git a/tests/PostcodePageTest.php b/tests/PostcodePageTest.php new file mode 100644 index 0000000000..f1a4a3e6cf --- /dev/null +++ b/tests/PostcodePageTest.php @@ -0,0 +1,76 @@ +createMySQLXMLDataSet(dirname(__FILE__).'/_fixtures/postcodepage.xml'); + } + + private function fetch_postcode_page($postcode, $options = array()) { + $args = array('pc' => $postcode); + $args = array_merge($args, $options); + + return $this->base_fetch_page( + $args, + 'postcode', + 'index.php', + '/postcode/' + ); + } + + public function testNoPostcodeGivesError() + { + $page = $this->fetch_postcode_page(''); + $this->assertContains('Please supply a postcode!', $page); + } + + public function testRedirectsForWestminsterOnly() + { + $page = $this->fetch_postcode_page('SW1A 1AA'); + $this->assertContains('Location: /mp/2/test_current-mp/test_westminster_constituency', $page); + } + + public function testShowsOptionsForScottishPostcode() + { + $page = $this->fetch_postcode_page('PH6 2DB'); + $this->assertContains('That postcode has multiple results', $page); + $this->assertContains('Test2 Current-MP', $page); + $this->assertContains('Test Current-MSP', $page); + $this->assertContains('Test Current-Regional-MSP', $page); + } + + public function testShowsOptionsForNIPostcode() + { + $page = $this->fetch_postcode_page('BT1 1AA'); + $this->assertContains('That postcode has multiple results', $page); + $this->assertContains('Test3 Current-MP', $page); + $this->assertContains('Test Current-MLA', $page); + } + + public function testRedirectsWithPolicySet() + { + $page = $this->fetch_postcode_page('SW1A 1AA', array('policy_set' => 'social')); + $this->assertContains('Location: /mp/2/test_current-mp/test_westminster_constituency/votes?policy=social', $page); + } + + public function testRedirectsWithPolicySetForScottishPostcode() + { + $page = $this->fetch_postcode_page('PH6 2DB', array('policy_set' => 'social')); + $this->assertContains('Location: /mp/3/test2_current-mp/test_scottish_westminster_constituency/votes?policy=social', $page); + } + + public function testRedirectsWithPolicySetForNIPostcode() + { + $page = $this->fetch_postcode_page('BT1 1AA', array('policy_set' => 'social')); + $this->assertContains('Location: /mp/6/test3_current-mp/test_ni_westminster_constituency/votes?policy=social', $page); + } +} diff --git a/tests/_fixtures/postcodepage.xml b/tests/_fixtures/postcodepage.xml new file mode 100644 index 0000000000..f7c582eba0 --- /dev/null +++ b/tests/_fixtures/postcodepage.xml @@ -0,0 +1,241 @@ + + + + + + + + + + + + + + 1 + Test Bill + http://example.com + unknown + 0 + 2013-14 + test_ + + + + + + + + + + + + + Test Westminster Constituency + test_constituency_key + Test Constituency Value + + + + + Test Scottish Westminster Constituency + 1 + 1983-00-00 + 9999-12-31 + 11 + + + Test NI Westminster Constituency + 1 + 1983-00-00 + 9999-12-31 + 12 + + + + + + + + + + + + + + + + + + + + + 1 + 1 + Test Westminster Constituency + + 2007-05-06 + 9999-12-31 + general_election + still_in_office + 2 + 2013-08-07 11:02:49 + + + 2 + 1 + Test Scottish Westminster Constituency + + 2007-05-06 + 9999-12-31 + general_election + still_in_office + 3 + 2013-08-07 11:02:49 + + + 3 + 4 + Test Holyrood Constituency + + 2007-05-06 + 9999-12-31 + general_election + still_in_office + 4 + 2013-08-07 11:02:49 + + + 4 + 4 + Test Scottish Region + + 2007-05-06 + 9999-12-31 + general_election + still_in_office + 5 + 2013-08-07 11:02:49 + + + 1 + 5 + Test NI Westminster Constituency + + 2007-05-06 + 9999-12-31 + general_election + still_in_office + 6 + 2013-08-07 11:02:49 + + + 6 + 3 + Test Stormont Constituency + + 2007-05-06 + 9999-12-31 + general_election + still_in_office + 7 + 2013-08-07 11:02:49 + + + + + Test + Current-MP + 2000-01-01 + 9999-12-31 + 2 + Mrs + + + Test2 + Current-MP + 2000-01-01 + 9999-12-31 + 3 + Mr + + + Test + Current-MSP + 2000-01-01 + 9999-12-31 + 4 + Mrs + + + Test + Current-Regional-MSP + 2000-01-01 + 9999-12-31 + 5 + Dr + + + Test3 + Current-MP + 2000-01-01 + 9999-12-31 + 6 + Miss + + + Test + Current-MLA + 2000-01-01 + 9999-12-31 + 7 + Mr + + + + + + + + + + + + + + + PH6 2DB + Test Scottish Westminster Constituency|Test Holyrood Constituency|Test Scottish Region + + + BT1 1AA + Test NI Westminster Constituency|Test Stormont Constituency + + + SW1A 1AA + Test Westminster Constituency + + + OX1 4LF + Another Westminster Constituency + + + + + + + + + + + + + + + + + + + + + + From 2377e2b02d9f3e26d746b8035040bfe53a47122e Mon Sep 17 00:00:00 2001 From: Struan Donald Date: Mon, 17 Apr 2017 10:54:44 +0100 Subject: [PATCH 8/9] avoid postcode lookups hitting mapit Add postcode_lookup rows in fixtures for tests that do postcode lookupts so that: * tests run offline * we don't have problems with mapit quotas --- tests/_fixtures/api.xml | 8 ++++++++ tests/_fixtures/postcode.xml | 4 ++++ 2 files changed, 12 insertions(+) diff --git a/tests/_fixtures/api.xml b/tests/_fixtures/api.xml index 6d327fbe95..24cb34ef6a 100644 --- a/tests/_fixtures/api.xml +++ b/tests/_fixtures/api.xml @@ -182,6 +182,14 @@ + + SW1A 1AA + Cities of London and Westminster + + + BT17 0XD + Belfast West|Belfast West + diff --git a/tests/_fixtures/postcode.xml b/tests/_fixtures/postcode.xml index 830f5d8e50..afffdade16 100644 --- a/tests/_fixtures/postcode.xml +++ b/tests/_fixtures/postcode.xml @@ -59,6 +59,10 @@ + + SW1A 1AA + Cities of London and Westminster + From 1a648e819d82ad402fcbbc867ea8aff0502c63d4 Mon Sep 17 00:00:00 2001 From: Struan Donald Date: Wed, 19 Apr 2017 15:27:33 +0100 Subject: [PATCH 9/9] add header to MP policy page if MP hasn't voted on policy It was unlikely that anyone would see this page before because it would only be linked to if the MP had voted. Now that we can redirect to this page it's more likely that it will be seen so tidy it up a bit. --- www/docs/mp/index.php | 1 + .../templates/html/mp/divisions.php | 20 ++++++++++++++++++- 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/www/docs/mp/index.php b/www/docs/mp/index.php index 54679d3c8e..c6f429e4eb 100644 --- a/www/docs/mp/index.php +++ b/www/docs/mp/index.php @@ -494,6 +494,7 @@ $divisions = new MySociety\TheyWorkForYou\Divisions($MEMBER, $positions, $policiesList); if ( $policyID ) { + $data['policy'] = $policiesList->getPolicyDetails($policyID); $data['policydivisions'] = $divisions->getMemberDivisionsForPolicy($policyID); } else { $data['policydivisions'] = $divisions->getAllMemberDivisionsByPolicy(); diff --git a/www/includes/easyparliament/templates/html/mp/divisions.php b/www/includes/easyparliament/templates/html/mp/divisions.php index 3895d98f0c..97e652ad1a 100644 --- a/www/includes/easyparliament/templates/html/mp/divisions.php +++ b/www/includes/easyparliament/templates/html/mp/divisions.php @@ -126,8 +126,26 @@ + +
+

+

.

+ + + Photo: + + + + + + + + +
+ +
-

This person has not voted on this policy.

+

has not voted on this policy.