From 285109c564148a0ed517d526bd8445457db5709a Mon Sep 17 00:00:00 2001 From: M Somerville Date: Wed, 21 Feb 2024 09:04:47 +0000 Subject: [PATCH] Remove party cohort code. (#1755) * Remove party cohort code. We store/use the figures returned externally. Some of the tests are therefore now only testing that what is stored in the fixture is displayed, rather than any calculations involved. --------- Co-authored-by: Alex Parsons --- classes/Member.php | 19 +- classes/PartyCohort.php | 456 ++------------------------- db/0023-remove-party-cohort.sql | 2 + db/schema.sql | 16 - scripts/dailyupdate | 9 - scripts/generate_party_positions.php | 10 - scripts/json2db.pl | 204 +++++++++++- scripts/quick-populate | 11 +- tests/PartyTest.php | 164 ++-------- tests/_fixtures/cohorts.xml | 113 ++++++- tests/_fixtures/divisions.xml | 77 ----- www/docs/mp/index.php | 2 +- 12 files changed, 346 insertions(+), 737 deletions(-) create mode 100644 db/0023-remove-party-cohort.sql delete mode 100644 scripts/generate_party_positions.php diff --git a/classes/Member.php b/classes/Member.php index e0d75c6b43..25f7261449 100644 --- a/classes/Member.php +++ b/classes/Member.php @@ -60,21 +60,6 @@ public function isDead() { } - - /** - * Cohort Key - * - * Gets a key that defines the periods and party a member should be compared against - * - * @return string of party and entry dates - */ - - public function cohortKey($house = HOUSE_TYPE_COMMONS) { - // get the hash_id for the cohort this member belongs to - $person_id = $this->person_id(); - return PartyCohort::getHashforPerson($person_id); - } - public function cohortPartyComparisonDirection() { // Is this MP and their cohort compared against the // first or last party they have? @@ -143,8 +128,10 @@ public function cohortParty($house = HOUSE_TYPE_COMMONS){ $party = $row["party"]; if ( $party == 'Labour/Co-operative' ) { $party = 'Labour'; + } elseif ($party == 'Sinn Féin') { + $party = 'Sinn Fein'; } - return $party; + return slugify($party); } else { return null; } diff --git a/classes/PartyCohort.php b/classes/PartyCohort.php index 1d23ced091..d599c928a7 100644 --- a/classes/PartyCohort.php +++ b/classes/PartyCohort.php @@ -15,97 +15,38 @@ class PartyCohort { private $db; - private $hash; - private $memberships; - private $absences; private $party; + private $hash; - public function __construct($hash, $pop_with_example = false) + public function __construct($person_id, $party) { - // $pop with example option will retrieve the relevant fields from - // an representative member of the cohort. // treat Labour and Labour/Co-operative the same as that's how // people view them and it'll confuse the results otherwise - $this->hash = $hash; + $this->party = $party; + $this->hash = "$person_id-$party"; $this->db = new \ParlDB; - $this->memberships = null; - $this->absences = null; - $this->party = null; - - if ($pop_with_example) { - $this->memberships = $this->getMemberships(); - $this->absences = $this->getAbsences(); - $this->party = $this->getParty(); - } } - public function getExamplePerson() + public function getAllPolicyPositions($policies) { - // get person_id for a typical member of this cohort - // given by definition all members have the same relevant properties - $row = $this->db->query( - "SELECT person_id from cohort_assignments where cohort_hash = :hash", - array(":hash" => $this->hash) - )->first(); + $positions = array(); - if ($row) { - return $row["person_id"]; - } else { - return null; + $party = $this->party; + if ($party == 'Independent') { + return $positions; } - } - - public function getMemberships() - { - // get start and left dates for this membership cohort - if (!is_null($this->memberships)) { - return $this->memberships; - }; - $house_id = 1; - $person_id = $this->getExamplePerson(); - $memberships = $this->db->query( - "SELECT person_id, entered_house as start_date, left_house as end_date - FROM member - WHERE house = :house_id and person_id = :person_id", - array(":person_id" => $person_id, ":house_id" => $house_id) - ); - return $memberships; - } - - public function getParty() - { - // get the party this cohort applies to - // currently this is the first non null value - if (!is_null($this->party)) { - return $this->party; - }; - $person_id = $this->getExamplePerson(); - $member = new Member(array('person_id' => $person_id)); - return $member->cohortParty(); - - } - public function getAbsences() - { - // get any known absences for this membership cohort - if (!is_null($this->absences)) { - return $this->absences; - }; - $person_id = $this->getExamplePerson(); - - $absences = $this->db->query( - "SELECT person_id, start_date, end_date - FROM known_absences - WHERE person_id = :person_id", - array(":person_id" => $person_id) - ); - return $absences; - } + foreach ($policies->getPolicies() as $policy_id => $policy_text) { + $data = $this->policy_position($policy_id); + if ($data === null) { + continue; + } - public function getAllPolicyPositions($policies) - { - $positions = $this->fetchPolicyPositionsByMethod($policies, "policy_position"); + $data['policy_id'] = $policy_id; + $data['desc'] = $policy_text; + $positions[$policy_id] = $data; + } return $positions; } @@ -132,365 +73,4 @@ public function policy_position($policy_id) return null; } } - - public function calculateAllPolicyPositions($policies) - { - $positions = $this->fetchPolicyPositionsByMethod($policies, "calculate_policy_position"); - - return $positions; - } - - public function calculate_policy_position($policy_id) - { - - // This could be done as a join but it turns out to be - // much slower to do that - $divisions = $this->db->query( - "SELECT division_id, division_date, policy_vote - FROM policydivisions - JOIN divisions USING(division_id) - WHERE policy_id = :policy_id - AND house = 'commons'", - array( - ':policy_id' => $policy_id - ) - ); - - $score = 0; - $max_score = 0; - $total_votes = 0; - $date_min = ''; - $date_max = ''; - $num_divisions = 0; - - $party = $this->getParty(); - $memberships = $this->getMemberships(); - $absences = $this->getAbsences(); - - $party_where = 'party = :party'; - $params = array( - ':party' => $party - ); - - if ($party == 'Labour') { - // need to get party id function - $party_where = '( party = :party OR party = :party2 )'; - $params = array( - ':party' => $party, - ':party2' => 'Labour/Co-operative' - ); - } - - foreach ($divisions as $division) { - - $division_id = $division['division_id']; - $date = $division['division_date']; - - // this bit makes sure we only include divisions in comparison that are part - // of the allowed cohort - $in_member_range = false; - $in_absence_range = false; - foreach ($memberships as $membership) { - $start_date = $membership["start_date"]; - $end_date = $membership["end_date"]; - if (($date <= $end_date) && ($date >= $start_date)) { - // this division occured within a comparison period - $in_member_range = true; - } - } - - foreach ($absences as $absence) { - $start_date = $absence["start_date"]; - $end_date = $absence["end_date"]; - if (($date <= $end_date) && ($date >= $start_date)) { - // this division occured within an absence period - $in_absence_range = true; - } - } - - // if not in range, or is in an absence range - // do not include this division in the comparison - if (!$in_member_range || $in_absence_range) { - continue; - } - - $weights = $this->get_vote_scores($division['policy_vote']); - $params[':division_id'] = $division_id; - $params[':division_date'] = $date; - - # join on the membership a person had at the time in question - # exclude any absent vote if someone was during a period of known absence - # this means a cohorts position is calculated based on people who were a member of the - # party at the time, and were not effectively absent - $votes = $this->db->query( - "SELECT count(*) as num_votes, vote - FROM persondivisionvotes - JOIN member ON persondivisionvotes.person_id = member.person_id - WHERE - $party_where - AND member.house = 1 - AND division_id = :division_id - AND member.entered_house <= :division_date - AND member.left_house >= :division_date - AND NOT - (persondivisionvotes.vote = 'absent' and persondivisionvotes.person_id in - ( SELECT person_id - FROM known_absences - WHERE start_date <= :division_date - AND end_date >= :division_date - ) - ) - - GROUP BY vote", - $params - ); - - $num_votes = 0; - foreach ($votes as $vote) { - $vote_dir = $vote['vote']; - if ($vote_dir == '') { - continue; - } - if ($vote_dir == 'tellno') { - $vote_dir = 'no'; - } - if ($vote_dir == 'tellaye') { - $vote_dir = 'aye'; - } - - $num_votes += $vote['num_votes']; - $score += ($vote['num_votes'] * $weights[$vote_dir]); - } - # For range, only care if there were results - if ($votes->rows()) { - if (!$date_min || $date_min > $date) { - $date_min = $date; - } - if (!$date_max || $date_max < $date) { - $date_max = $date; - } - $num_divisions++; - } - - $total_votes += $num_votes; - $max_score += $num_votes * max(array_values($weights)); - } - - if ($total_votes == 0) { - return null; - } - - // this implies that all the divisions in the policy have a policy - // position of absent so we set weight to -1 to indicate we can't - // really say what the parties position is. - if ($max_score == 0) { - $weight = -1; - } else { - $weight = 1 - ($score / $max_score); - } - $score_desc = score_to_strongly($weight); - - return [ - 'score' => $weight, - 'position' => $score_desc, - 'divisions' => $num_divisions, - 'date_min' => $date_min, - 'date_max' => $date_max, - ]; - } - - public function cache_position($position) - { - $this->db->query( - "REPLACE INTO partypolicy - (party, house, policy_id, score, divisions, date_min, date_max) - VALUES (:party, 1, :policy_id, :score, :divisions, :date_min, :date_max)", - array( - ':score' => $position['score'], - ':party' => $this->hash, - ':policy_id' => $position['policy_id'], - ':divisions' => $position['divisions'], - ':date_min' => $position['date_min'], - ':date_max' => $position['date_max'], - ) - ); - } - - private function fetchPolicyPositionsByMethod($policies, $method) - { - $positions = array(); - - $party = $this->getParty(); - if ($party == 'Independent') { - return $positions; - } - - foreach ($policies->getPolicies() as $policy_id => $policy_text) { - $data = $this->$method($policy_id); - if ($data === null) { - continue; - } - - $data['policy_id'] = $policy_id; - $data['desc'] = $policy_text; - $positions[$policy_id] = $data; - } - - return $positions; - } - - private function get_vote_scores($vote) - { - $absent = 1; - $both = 1; - $agree = 10; - - if (stripos($vote, '3') !== false) { - $agree = 50; - $absent = 25; - $both = 25; - } - - $scores = array( - 'absent' => $absent, - 'both' => $both - ); - - if (stripos($vote, 'aye') !== false) { - $scores['aye'] = $agree; - $scores['no'] = 0; - } else if (stripos($vote, 'no') !== false) { - $scores['no'] = $agree; - $scores['aye'] = 0; - } else { - $scores['both'] = 0; - $scores['absent'] = 0; - $scores['no'] = 0; - $scores['aye'] = 0; - } - - return $scores; - } - - public static function getCohorts() - { - $db = new \ParlDB; - $cohort_list = $db->query( - "SELECT DISTINCT cohort_hash FROM cohort_assignments" - ); - - $cohorts = array(); - foreach ($cohort_list as $row) { - $cohort = $row['cohort_hash']; - $cohorts[] = $cohort; - } - - return $cohorts; - } - - public static function getCohortQuery(){ - $membership_query = ' - select member_periods.person_id as person_id, - md5(concat(start_party, "|", membership_key, "|", IFNULL(absence_key,""))) as cohort_hash - from - (select - person_id, - REPLACE(COALESCE(m2party), "/Co-operative", "") as "start_party", - GROUP_CONCAT(concat(entered_house,":", left_house) ORDER BY entered_house) as "membership_key" - from - member - -inner join -( -select party m2party, person_id m2pid -from member -where - house = :house and entered_house > :cut_off -and entered_house = (select min(entered_house) from member m3 where house=:house and entered_house > :cut_off and member.person_id=m3.person_id) -) m2 -ON member.person_id = m2.m2pid - - where - house = :house and entered_house > :cut_off - group by person_id - order by person_id - )as member_periods - left join - (select - person_id, - GROUP_CONCAT(concat(start_date,":", end_date)) as "absence_key" - from - known_absences - group by person_id - order by person_id, start_date - ) as absences on member_periods.person_id = absences.person_id'; - return $membership_query; - } - - - public static function getHashforPerson($person_id){ - // given a person id, return the hash for that cohort - $db = new \ParlDB; - $row = $db->query("SELECT cohort_hash - FROM cohort_assignments - WHERE person_id = :person_id", - array(":person_id" => $person_id))->first(); - - if ($row) { - return $row["cohort_hash"]; - } else { - return null; - } - } - - public static function calculatePositions($quiet=true){ - // get current hashes available - $cohorts = PartyCohort::getCohorts(); - $db = new \ParlDB; - $policies = new Policies; - $n_cohorts = count($cohorts); - - // iterate through all hashes and create policy positions - $cohort_count = 0; - foreach ( $cohorts as $cohort ) { - - $cohort = new PartyCohort($cohort, true); - - $positions = $cohort->calculateAllPolicyPositions($policies); - - $cohort_count++; - - $db->conn->beginTransaction(); - foreach ( $positions as $position ) { - $cohort->cache_position( $position ); - } - $db->conn->commit(); - if (!$quiet) { - print("$cohort_count/$n_cohorts\n"); - } - } - - } - - public static function populateCohorts() - { - // create the cohort_assignments table from the query - $db = new \ParlDB; - $house = 1; - $cut_off = '1997-01-01'; - - $membership_query = self::getCohortQuery(); - $insert_query = "INSERT INTO cohort_assignments (person_id, cohort_hash) $membership_query"; - - $db->query("DELETE FROM cohort_assignments"); - - $q = $db->query( - $insert_query, - array( - ':house' => $house, - ":cut_off" => $cut_off - ) - ); - - } } diff --git a/db/0023-remove-party-cohort.sql b/db/0023-remove-party-cohort.sql new file mode 100644 index 0000000000..eab6d9054a --- /dev/null +++ b/db/0023-remove-party-cohort.sql @@ -0,0 +1,2 @@ +DROP TABLE `known_absences`; +DROP TABLE `cohort_assignments`; diff --git a/db/schema.sql b/db/schema.sql index 006a23f1bf..e78e4c426d 100644 --- a/db/schema.sql +++ b/db/schema.sql @@ -554,19 +554,3 @@ CREATE TABLE `topic_epobjects` ( `epobject_id` int(11) NOT NULL, UNIQUE KEY `topic_object` (`topic_key`, `epobject_id`) ); - -CREATE TABLE `known_absences` ( - `id` int(11) NOT NULL auto_increment, - `person_id` int(11) NOT NULL default '0', - `start_date` date NOT NULL default '1000-01-01', - `end_date` date NOT NULL default '9999-12-31', - `desc` varchar(100) NOT NULL default '', - PRIMARY KEY (`id`), - KEY `person_id` (`person_id`) -); - -CREATE TABLE `cohort_assignments`( - `person_id` int(11) NOT NULL default '0', - `cohort_hash` varchar(100) NOT NULL default '', - PRIMARY KEY (`person_id`) -); diff --git a/scripts/dailyupdate b/scripts/dailyupdate index 867cc93225..518238a9ec 100755 --- a/scripts/dailyupdate +++ b/scripts/dailyupdate @@ -30,12 +30,3 @@ system './mpinfoin.pl'; # update individual division votes system "./json2db.pl >> $cron_log"; - -# update party positions -system "php ./generate_party_positions.php >> $cron_log"; - -#unless ($staging) { -# chdir $pwmembers; -# system 'svn commit -m "apply updates from twfy"'; -#} - diff --git a/scripts/generate_party_positions.php b/scripts/generate_party_positions.php deleted file mode 100644 index eacbe58ce6..0000000000 --- a/scripts/generate_party_positions.php +++ /dev/null @@ -1,10 +0,0 @@ -new->latin1; @@ -43,21 +51,49 @@ my $personinfo_check = $dbh->prepare("SELECT data_value from personinfo where data_key = ? and person_id = ?"); my $strong_for_policy_check = $dbh->prepare("SELECT count(*) as strong_votes FROM persondivisionvotes JOIN policydivisions USING (division_id) WHERE policy_id = ? AND person_id = ? AND policy_vote LIKE '%3'"); +my $divisioncheck = $dbh->prepare("SELECT division_title, gid, yes_text, no_text, yes_total, no_total, absent_total, both_total, majority_vote FROM divisions WHERE division_id = ?"); +my $divisionadd = $dbh->prepare("INSERT INTO divisions (division_id, house, division_title, yes_text, no_text, division_date, division_number, gid, yes_total, no_total, absent_total, both_total, majority_vote) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)"); +my $divisionupdate = $dbh->prepare("UPDATE divisions SET gid = ?, division_title = ?, yes_text = ?, no_text = ?, yes_total = ?, no_total = ?, absent_total = ?, both_total = ?, majority_vote = ? WHERE division_id = ?"); + +my $votecheck = $dbh->prepare("SELECT person_id, vote FROM persondivisionvotes WHERE division_id = ?"); +my $voteadd = $dbh->prepare("INSERT INTO persondivisionvotes (person_id, division_id, vote) VALUES (?, ?, ?)"); +my $voteupdate= $dbh->prepare("UPDATE persondivisionvotes SET vote = ? WHERE person_id = ? AND division_id = ?"); + +my $partypolicy_replace = $dbh->prepare("REPLACE INTO partypolicy + (party, house, policy_id, score, divisions, date_min, date_max) + VALUES (?, 1, ?, ?, ?, ?, ?)"); + $motion_count = $policy_count = $align_count = 0; my @policyids = fetch_policies(); -my $ua = LWP::UserAgent->new; -$ua->timeout(10); # 10 second timeout +sub get_url { + # simplify retrying urls if they fail + my ($url, $retries) = @_; + my $ua = LWP::UserAgent->new; + $ua->timeout(10); # 10 second timeout + my $response = $ua->get($url); + if ($response->is_success) { + return $response->decoded_content; + } else { + if ($retries > 0) { + sleep(30); + say "retrying $url" if $verbose; + return get_url($url, $retries - 1); + } else { + return undef; + } + } +} foreach my $dreamid ( @policyids ) { + say "fetching data for $dreamid" if $verbose; my $policy_url = mySociety::Config::get('TWFY_VOTES_URL') . '/twfy-compatible/popolo/' . $dreamid . '.json'; - my $response = $ua->get($policy_url); - unless ($response->is_success) { - warn "no json file for policy $dreamid at $policy_url"; + my $policy_json = get_url($policy_url, 3); + unless ($policy_json) { + warn "no json file for policy $dreamid at $policy_url\n"; next; } - my $policy_json = $response->decoded_content; my $policy = $json->decode($policy_json); my $curr_policy = $dbh->selectrow_hashref($policycheck, {}, $dreamid); @@ -73,6 +109,10 @@ process_policydivisions($policy->{aspects}, $dreamid); say "processing alignments for $dreamid" if $verbose; process_alignments($policy->{alignments}, $dreamid); + if ( $dev_populate ) { + say "Dev mode - Populating divisions for $dreamid" if $verbose; + process_motions($policy, $dreamid); + } } print "parsed $policy_count policies, $motion_count divisions, and $align_count alignments from JSON\n"; @@ -81,7 +121,7 @@ sub fetch_policies { my $policies_url = mySociety::Config::get('TWFY_VOTES_URL') . '/policies/commons/active/all.json'; - my $policies_json = get($policies_url); + my $policies_json = get_url($policies_url, 3); my $policies = $json->decode($policies_json); my @ids; @@ -173,6 +213,8 @@ sub process_policydivisions { sub process_alignments { my ($alignments, $dreamid) = @_; + # Set AutoCommit off + $dbh->{AutoCommit} = 0; foreach (@$alignments) { $align_count++; say $align_count if $verbose && $align_count % 100 == 0; @@ -189,5 +231,151 @@ sub process_alignments { my $val = $_->{$term->[1]}; $personinfo_set->execute($person_id, $pw_id, $val, $val); } + + unless ($_->{no_party_comparision}) { + my $hash = "$person_id-$_->{comparison_party}"; + my $divisions = $_->{count_present} + $_->{count_absent}; + my $start_date = "$_->{start_year}-00-00"; + my $end_date = "$_->{end_year}-00-00"; + $partypolicy_replace->execute( + $hash, $dreamid, $_->{comparison_distance_from_policy}, + $divisions, $start_date, $end_date); + } + } + $dbh->commit(); + # Set AutoCommit on + $dbh->{AutoCommit} = 1; } + + + +sub process_motions { + my ($policy, $dreamid) = @_; + # Set AutoCommit off + $dbh->{AutoCommit} = 0; + for my $motion ( @{ $policy->{aspects} } ) { + $motion_count++; + if ($verbose && $motion_count % 10 == 0){ + print("$motion_count\n"); + }; + my ($motion_num) = $motion->{motion}->{id} =~ /pw-\d+-\d+-\d+-(\d+)/; + my ($house) = $motion->{motion}->{organization_id} =~ /uk\.parliament\.(\w+)/; + + my $sources = $motion->{motion}->{sources}; + my $gid = ''; + foreach my $source (@$sources) { + if ( defined $source->{gid} ) { + $gid = $source->{gid}; + } + } + + my $motion_id = $motion->{motion}->{id}; + my $text = $motion->{motion}->{text}; + + my $curr_division = $dbh->selectrow_hashref($divisioncheck, {}, $motion_id); + if ( $curr_division ) { + $curr_division->{yes_text} ||= ''; + $curr_division->{no_text} ||= ''; + } + + + my $yes_text = ''; + my $no_text = ''; + if ( $motion->{motion}->{actions} ) { + $yes_text = $motion->{motion}->{actions}->{yes}; + $no_text = $motion->{motion}->{actions}->{no}; + } + + my $totals = { + yes => 0, + no => 0, + absent => 0, + both => 0, + }; + my $majority_vote = ''; + + if ( $motion->{motion}->{vote_events}->[0]->{counts} ) { + for my $count ( @{ $motion->{motion}->{vote_events}->[0]->{counts} } ) { + $totals->{$count->{option}} = $count->{value}; + } + + if ($totals->{yes} > $totals->{no}) { + $majority_vote = 'aye'; + } else { + $majority_vote = 'no'; + } + } + + # Ignore tellers in totals + $totals->{yes} -= grep { $_->{option} =~ /tellaye/ } @{ $motion->{motion}->{ vote_events }->[0]->{votes} }; + $totals->{no} -= grep { $_->{option} =~ /tellno/ } @{ $motion->{motion}->{ vote_events }->[0]->{votes} }; + + if ( !defined $curr_division ) { + my $r = $divisionadd->execute($motion_id, $house, $motion->{motion}->{text}, $yes_text, $no_text, $motion->{motion}->{date}, $motion_num, $gid, $totals->{yes}, $totals->{no}, $totals->{absent}, $totals->{both}, $majority_vote); + unless ( $r > 0 ) { + warn "problem creating division $motion_id, skipping motions\n"; + next; + } + } elsif ( $curr_division->{division_title} ne $text || + $curr_division->{gid} ne $gid || + $curr_division->{yes_text} ne $yes_text || + $curr_division->{no_text} ne $no_text || + $curr_division->{yes_total} ne $totals->{yes} || + $curr_division->{no_total} ne $totals->{no} || + $curr_division->{absent_total} ne $totals->{absent} || + $curr_division->{both_total} ne $totals->{both} || + $curr_division->{majority_vote} ne $majority_vote + ) { + my $r = $divisionupdate->execute($gid, $text, $yes_text, $no_text, $totals->{yes}, $totals->{no}, $totals->{absent}, $totals->{both}, $majority_vote, $motion_id); + unless ( $r > 0 ) { + warn "problem updating division $motion_id from $curr_division->{division_title} to $text AND $curr_division->{gid} to $gid\n"; + } + } + + + my $curr_votes = $dbh->selectall_hashref($votecheck, 'person_id', {}, $motion_id); + + for my $vote ( @{ $motion->{motion}->{ vote_events }->[0]->{votes} } ) { + my $mp_id_num; + $mp_id_num = $vote->{id}; + $mp_id_num =~ s:uk.org.publicwhip/person/::; + next unless $mp_id_num; + if ( $mp_id_num !~ /^[1-9]\d+$/ ) { + print "$mp_id_num doesn't look like a valid person id - skipping vote for $motion_id - " . ($dreamid || "") . "\n"; + next; + } + + # if we've seen this motion before then don't process it, however we want + # to make sure that the strong vote processing below happens so we still + # need to look at all the votes, just not update the details of them in + # the database + if ( !$motions_seen{$motion_id} ) { + $vote_count++; + + if ( !defined $curr_votes->{$mp_id_num} ) { + $voteadd->execute($mp_id_num, $motion_id, $vote->{option}); + $curr_votes->{$mp_id_num} = { vote => $vote->{option}}; + } elsif ( $curr_votes->{$mp_id_num}->{vote} ne $vote->{option} ) { + # because we probably want to know if this ever happens + print "updating $motion_id vote for $mp_id_num from " . $curr_votes->{$mp_id_num}->{vote} . " to " . $vote->{option} . "\n"; + my $r = $voteupdate->execute($vote->{option}, $mp_id_num, $motion_id); + unless ( $r > 0 ) { + warn "problem updating $motion_id vote for $mp_id_num from " . $curr_votes->{$mp_id_num}->{vote} . " to " . $vote->{option} . "\n" + . DBI->errstr . "\n"; + } + } + } + } + + # some divisions are in more than one policy and we want to take note of + # this so we can skip processing of them + if ( !$motions_seen{$motion_id} ) { + $motions_seen{$motion_id} = 1; + } + + } + $dbh->commit(); + # Set AutoCommit on + $dbh->{AutoCommit} = 1; +} \ No newline at end of file diff --git a/scripts/quick-populate b/scripts/quick-populate index 27e60d1993..9f54952181 100755 --- a/scripts/quick-populate +++ b/scripts/quick-populate @@ -9,9 +9,7 @@ echo 'Downloading minimal data' rsync -az --progress --exclude '.svn' --exclude 'tmp/' --relative data.theyworkforyou.com::parldata/scrapedxml/debates/debates2022-01* data/pwdata rsync -az --progress --exclude '.svn' --exclude 'tmp/' --relative data.theyworkforyou.com::parldata/scrapedjson data/pwdata rsync -az --progress --exclude '.svn' --exclude 'tmp/' --relative data.theyworkforyou.com::parldata/people.json data/pwdata -rsync -az --progress --exclude '.svn' --exclude 'tmp/' --relative data.theyworkforyou.com::parldata/scrapedxml/divisionsonly/divisions2022-01* data/pwdata rsync -az --progress --exclude '.svn' --exclude 'tmp/' --relative data.theyworkforyou.com::parldata/scrapedxml/regmem data/pwdata - echo 'Downloading minimal Senedd' rsync -az --progress --exclude '.svn' --exclude 'tmp/' --relative data.theyworkforyou.com::parldata/scrapedxml/senedd/en/senedd2022-01* data/pwdata rsync -az --progress --exclude '.svn' --exclude 'tmp/' --relative data.theyworkforyou.com::parldata/scrapedxml/senedd/cy/senedd2022-01* data/pwdata @@ -22,8 +20,8 @@ rsync -az --progress --exclude '.svn' --exclude 'tmp/' --relative data.theyworkf echo 'Load people into database' scripts/load-people -echo 'Load divisions/policies from json' -scripts/json2db.pl --verbose +echo 'Load policies and votes from twfy-votes' +scripts/json2db.pl --verbose --dev-populate echo 'Load Jan 2022 debates and divisions' scripts/xml2db.pl --debates --wales --scotland --all @@ -32,9 +30,6 @@ scripts/xml2db.pl --debates --wales --scotland --all echo 'Importing MP extra info for voting comparison' scripts/mpinfoin.pl publicwhip -echo 'Generate party positions on issues' -php scripts/generate_party_positions.php - echo 'Generate search index' search/index.pl all -echo 'Quick setup completed' \ No newline at end of file +echo 'Quick setup completed' diff --git a/tests/PartyTest.php b/tests/PartyTest.php index b25fe94a07..f018e951f9 100644 --- a/tests/PartyTest.php +++ b/tests/PartyTest.php @@ -32,67 +32,16 @@ public function testLoad() } - public function testSameDateRangeSameCohort() - { - // Person ID 1 and 2 should have the same cohort - // They both belong to A party and have the same date ranges - - $member = $this->getMemberFromPersonId(1); - $cohortkeya = $member->cohortKey(); - - $member = $this->getMemberFromPersonId(2); - $cohortkeyb = $member->cohortKey(); - $this->assertEquals($cohortkeya, $cohortkeyb); - } - - public function testdifferentDatesDifferentCohorts() - { - //A third MP, but with a known absence added, end up in different cohorts. - //Person ID 3 has a known absence, and so should be in a different cohort to 1. - - MySociety\TheyWorkForYou\PartyCohort::populateCohorts(); - MySociety\TheyWorkForYou\PartyCohort::calculatePositions(); - - $query = MySociety\TheyWorkForYou\PartyCohort::getCohortQuery(); - - $member = $this->getMemberFromPersonId(1); - $cohortkeya = $member->cohortKey(); - - $member = $this->getMemberFromPersonId(3); - $cohortkeyb = $member->cohortKey(); - - $this->assertNotEquals($cohortkeya, $cohortkeyb); - } - - public function testdifferentPartiesDifferentCohorts() - { - //Two MPs of different parties with the same date range end up in different cohorts. - //Person ID 1 and person ID 4 should have different cohorts. - - MySociety\TheyWorkForYou\PartyCohort::populateCohorts(); - MySociety\TheyWorkForYou\PartyCohort::calculatePositions(); - - $member = $this->getMemberFromPersonId(1); - $cohortkeya = $member->cohortKey(); - - $member = $this->getMemberFromPersonId(4); - $cohortkeyb = $member->cohortKey(); - $this->assertNotEquals($cohortkeya, $cohortkeyb); - } - public function testTwoLabourPartiesAreOneParty() { - //A labour and labour/coop mp with the same date ranges end up in the same cohort. + //A labour and labour/coop mp end up in the same cohort. //Person ID 5 and 6 have the same cohort. - MySociety\TheyWorkForYou\PartyCohort::populateCohorts(); - MySociety\TheyWorkForYou\PartyCohort::calculatePositions(); - $member = $this->getMemberFromPersonId(5); - $cohortkeya = $member->cohortKey(); + $cohortkeya = $member->cohortParty(); $member = $this->getMemberFromPersonId(6); - $cohortkeyb = $member->cohortKey(); + $cohortkeyb = $member->cohortParty(); $this->assertEquals($cohortkeya, $cohortkeyb); } @@ -101,12 +50,9 @@ public function testPartyChangerComparison() //An MP who changes party should have their first party as their comparison party. // Person id 7, has changed party but should have 'A Party' not 'C Party' as their comparison party. - MySociety\TheyWorkForYou\PartyCohort::populateCohorts(); - MySociety\TheyWorkForYou\PartyCohort::calculatePositions(); - $member = $this->getMemberFromPersonId(7); $comparison_party = $member->cohortParty(); - $this->assertEquals($comparison_party, "A Party"); + $this->assertEquals($comparison_party, "a-party"); } @@ -114,12 +60,9 @@ public function testManualPartyChangerComparison() { // Person 10172 has changed party but has a manual override so should use their last party - MySociety\TheyWorkForYou\PartyCohort::populateCohorts(); - MySociety\TheyWorkForYou\PartyCohort::calculatePositions(); - $member = $this->getMemberFromPersonId(10172); $comparison_party = $member->cohortParty(); - $this->assertEquals($comparison_party, "I Party"); + $this->assertEquals($comparison_party, "i-party"); } @@ -127,12 +70,9 @@ public function testSpeakerPartyChangerComparison() { // Person 25 is the speaker, and so doesn't get their former party comparison. - MySociety\TheyWorkForYou\PartyCohort::populateCohorts(); - MySociety\TheyWorkForYou\PartyCohort::calculatePositions(); - $member = $this->getMemberFromPersonId(25); $comparison_party = $member->cohortParty(); - $this->assertEquals($comparison_party, "Speaker"); + $this->assertEquals($comparison_party, "speaker"); } @@ -141,39 +81,20 @@ public function testComparisonDateRange() //A policy comparison for a policy with divisions that include 1 outside an mps memberships will not include that division. // Person 1 should end up with a policy with a comparison period for policy 1120 starting in 2002 and ending in 2006 - MySociety\TheyWorkForYou\PartyCohort::populateCohorts(); - MySociety\TheyWorkForYou\PartyCohort::calculatePositions(); - $positions = $this->positionsForPersonID(1); - $specific_policy = $positions[1120]; - - $this->assertEquals($specific_policy["date_min"], "2002-01-01"); - $this->assertEquals($specific_policy["date_max"], "2006-05-01"); + $this->assertEquals($specific_policy["date_min"], "2002-00-00"); + $this->assertEquals($specific_policy["date_max"], "2006-00-00"); } public function testComparisonDateLongerRange() { //For same policy as testComparisonDateExclusion, someone with a longer range should have a different date_min // Person 8 starts from 1995, so should end with a comparison period for policy 1120 in 1998. - - MySociety\TheyWorkForYou\PartyCohort::populateCohorts(); - MySociety\TheyWorkForYou\PartyCohort::calculatePositions(); - - $member = $this->getMemberFromPersonId(1); - $cohortkeya = $member->cohortKey(); - - $member = $this->getMemberFromPersonId(8); - $cohortkeyb = $member->cohortKey(); - - $this->assertNotEquals($cohortkeya, $cohortkeyb); - $positions = $this->positionsForPersonID(8); - $specific_policy = $positions['1120']; - - $this->assertEquals($specific_policy["date_min"], "1998-01-01"); - $this->assertEquals($specific_policy["date_max"], "2006-05-01"); + $this->assertEquals($specific_policy["date_min"], "1998-00-00"); + $this->assertEquals($specific_policy["date_max"], "2006-00-00"); } @@ -183,15 +104,10 @@ public function testComparisonDateExclusions() // Person 3 has a known absence during 2006, and so their cohort will only cover the one division for 1120 - MySociety\TheyWorkForYou\PartyCohort::populateCohorts(); - MySociety\TheyWorkForYou\PartyCohort::calculatePositions(); - $positions = $this->positionsForPersonID(3); - $specific_policy = $positions['1120']; - - $this->assertEquals($specific_policy["date_min"], "2002-01-01"); - $this->assertEquals($specific_policy["date_max"], "2002-01-01"); + $this->assertEquals($specific_policy["date_min"], "2002-00-00"); + $this->assertEquals($specific_policy["date_max"], "2002-00-00"); } public function testPartyChangeHistoryRobustness(){ @@ -202,9 +118,6 @@ public function testPartyChangeHistoryRobustness(){ //10 and 12 vote the same way of on the division for policy 1124 in 2014, 11 and 13 vote the opposite way. //The cohort position should be the same for the cohorts person 10, 12 and 13 belong to. - MySociety\TheyWorkForYou\PartyCohort::populateCohorts(); - MySociety\TheyWorkForYou\PartyCohort::calculatePositions(); - $positionsa = $this->positionsForPersonID(10)[1124]; $positionsb = $this->positionsForPersonID(12)[1124]; $positionsc = $this->positionsForPersonID(13)[1124]; @@ -233,9 +146,6 @@ public function testCalcAndGetPolicyPositions() # Calculate all party positions # For an A Party MP (1), get their cohort's preference - MySociety\TheyWorkForYou\PartyCohort::populateCohorts(); - MySociety\TheyWorkForYou\PartyCohort::calculatePositions(); - $positions = $this->positionsForPersonID(1); $comparison = array('1113' => $positions['1113'], '810' => $positions['810']); @@ -246,8 +156,8 @@ public function testCalcAndGetPolicyPositions() 'score' => '0.5', 'desc' => 'an equal number of electors per parliamentary constituency', 'divisions' => 1, - 'date_min' => '2021-02-11', - 'date_max' => '2021-02-11', + 'date_min' => '2021-00-00', + 'date_max' => '2021-00-00', 'policy_id' => 1113 ), '810' => array( @@ -255,8 +165,8 @@ public function testCalcAndGetPolicyPositions() 'score' => '0.5', 'desc' => 'greater regulation of gambling', 'divisions' => 1, - 'date_min' => '2021-02-11', - 'date_max' => '2021-02-11', + 'date_min' => '2021-00-00', + 'date_max' => '2021-00-00', 'policy_id' => 810 ) ); @@ -269,9 +179,6 @@ public function testCalcAndGetPolicyPositionsForIndependents() # Independent MPs should not have policy positions # MP 14 is an independent MP - MySociety\TheyWorkForYou\PartyCohort::populateCohorts(); - MySociety\TheyWorkForYou\PartyCohort::calculatePositions(); - $positions = $this->positionsForPersonID(14); $this->assertEquals(array(), $positions); @@ -282,12 +189,9 @@ public function testGetRestrictedPositions() # Person 1 is a member of party A. # Party A should have no policies for polices 6667 - MySociety\TheyWorkForYou\PartyCohort::populateCohorts(); - MySociety\TheyWorkForYou\PartyCohort::calculatePositions(); - $member = $this->getMemberFromPersonId(1); - $cohortkey = $member->cohortKey(); - $cohort = new MySociety\TheyWorkForYou\PartyCohort($cohortkey, true); + $party = $member->cohortParty(); + $cohort = new MySociety\TheyWorkForYou\PartyCohort(1, $party); $policies = new MySociety\TheyWorkForYou\Policies(6667); $positions = $cohort->getAllPolicyPositions($policies); $expectedPositions = array(); @@ -301,9 +205,6 @@ public function testCalculatePositionsPolicyAbsent() # were absent (-1) # Person 6 is a coop MP - MySociety\TheyWorkForYou\PartyCohort::populateCohorts(); - MySociety\TheyWorkForYou\PartyCohort::calculatePositions(); - $data = $this->positionsForPersonID(6); $data = $data['996']; @@ -319,9 +220,6 @@ public function testAbsentVoteDoesNotEffectPolicyDuringAbsence() # Person 2 (in party A) should have a cohort policy score # that is just (aye) (0) - MySociety\TheyWorkForYou\PartyCohort::populateCohorts(); - MySociety\TheyWorkForYou\PartyCohort::calculatePositions(); - $data = $this->positionsForPersonID(2); $data = $data['363']; @@ -341,8 +239,8 @@ public function testGetAllParties() private function positionsForPersonID($person_id) { $member = $this->getMemberFromPersonId($person_id); - $cohortkey = $member->cohortKey(); - $cohort = new MySociety\TheyWorkForYou\PartyCohort($cohortkey, true); + $party = $member->cohortParty(); + $cohort = new MySociety\TheyWorkForYou\PartyCohort($person_id, $party); $policies = new MySociety\TheyWorkForYou\Policies(); $positions = $cohort->getAllPolicyPositions($policies); @@ -351,25 +249,13 @@ private function positionsForPersonID($person_id) private function getMemberFromPersonId($person_id) { - $db = new \ParlDB; - $row = $db->query( - "select member_id from member where person_id = :person_id order by entered_house desc", - array(":person_id" => $person_id) - )->first(); - - if ($row) { - return new MySociety\TheyWorkForYou\Member($row); - } else { - return null; - } + return new MySociety\TheyWorkForYou\Member([ "person_id" => $person_id ]); } public function testMPPartyPolicyTextWhenDiffers() { // Checks that an MP that differs from party gets the 'sometimes differs from their party' on the profile page - MySociety\TheyWorkForYou\PartyCohort::populateCohorts(); - MySociety\TheyWorkForYou\PartyCohort::calculatePositions(); $page = $this->fetch_page(array('pid' => 15, 'url' => '/mp/15/test_mp_g_party_1/test_westminster_constituency')); $this->assertStringContainsString('Test MP G Party 1', $page); $this->assertStringContainsString('is a G Party MP', $page); @@ -379,8 +265,6 @@ public function testMPPartyPolicyTextWhenDiffers() public function testSingleMemberPartyPolicyText() { // this test checks it doesn't say they are an X party MP when they are the only MP of that party - MySociety\TheyWorkForYou\PartyCohort::populateCohorts(); - MySociety\TheyWorkForYou\PartyCohort::calculatePositions(); $page = $this->fetch_page(array('pid' => 7, 'url' => '/mp/7/test_mp_g/test_westminster_constituency')); $this->assertStringContainsString('Test MP G', $page); $this->assertStringNotContainsString('is a B Party MP', $page); @@ -390,8 +274,6 @@ public function testMPPartyPolicyWherePartyMissingPositions() { // When an MP has votes, but there is no broader party policy to compare it to // this goes down a funnel that shows the votes, but does not make the comparison to party. - MySociety\TheyWorkForYou\PartyCohort::populateCohorts(); - MySociety\TheyWorkForYou\PartyCohort::calculatePositions(); $page = $this->fetch_page(array('pid' => 4, 'url' => '/mp/4/test_mp_d/test_westminster_constituency')); $this->assertStringContainsString('Test MP D', $page); $this->assertStringContainsString('This is a random selection of Mrs Test MP D’s votes', $page); @@ -402,8 +284,6 @@ public function testMPPartyPolicyWherePartyMissingPositions() public function testMPPartyPolicyTextWhenAgrees() { // Test when an MP mostly agrees with their party, as MP G Party 2 does with party G - MySociety\TheyWorkForYou\PartyCohort::populateCohorts(); - MySociety\TheyWorkForYou\PartyCohort::calculatePositions(); $page = $this->fetch_page(array('pid' => 16, 'url' => '/mp/16/test_mp_g_party_2/test_westminster_constituency')); $this->assertStringContainsString('Test MP G Party 2', $page); @@ -414,8 +294,6 @@ public function testMPPartyPolicyTextWhenAgrees() public function testCrossPartyDisclaimer() { // Test if the cross party disclaimer is there - MySociety\TheyWorkForYou\PartyCohort::populateCohorts(); - MySociety\TheyWorkForYou\PartyCohort::calculatePositions(); $page = $this->fetch_page(array('pagetype' => 'votes', 'pid' => 7, 'url' => '/mp/7/test_mp_g/test_westminster_constituency/votes')); $this->assertStringContainsString('Test MP G', $page); $this->assertStringContainsString('In the votes below they are compared to their original party', $page); diff --git a/tests/_fixtures/cohorts.xml b/tests/_fixtures/cohorts.xml index 27ddd43ac8..8821dc5ca7 100644 --- a/tests/_fixtures/cohorts.xml +++ b/tests/_fixtures/cohorts.xml @@ -745,15 +745,6 @@ - - - 1 - 3 - 2006-01-01 - 2007-01-01 - health - - 15 @@ -1064,7 +1055,107 @@ aye - + + + 1 + 1-a-party + 1120 + 0.5 + 10 + 2002-00-00 + 2006-00-00 + + + 1 + 3-a-party + 1120 + 0.5 + 10 + 2002-00-00 + 2002-00-00 + + + 1 + 8-a-party + 1120 + 0.5 + 10 + 1998-00-00 + 2006-00-00 + + + 1 + 10-d-party + 1124 + 0.5 + 10 + 2022-01-01 + 2023-01-01 + + + 1 + 12-e-party + 1124 + 0.5 + 10 + 2022-01-01 + 2023-01-01 + + + 1 + 13-e-party + 1124 + 0.5 + 10 + 2022-01-01 + 2023-01-01 + + + 1 + 1-a-party + 1113 + 0.5 + 1 + 2021-00-00 + 2021-00-00 + + + 1 + 1-a-party + 810 + 0.5 + 1 + 2021-00-00 + 2021-00-00 + + + 1 + 6-labour + 996 + -1 + 10 + 2022-01-01 + 2023-01-01 + + + 1 + 2-a-party + 363 + 0 + 10 + 2022-01-01 + 2023-01-01 + + + 1 + 15-g-party + 1113 + 0.87 + 1 + 2022-01-01 + 2023-01-01 + + @@ -1075,4 +1166,4 @@ - \ No newline at end of file + diff --git a/tests/_fixtures/divisions.xml b/tests/_fixtures/divisions.xml index 5f09034d48..3dfe55a290 100644 --- a/tests/_fixtures/divisions.xml +++ b/tests/_fixtures/divisions.xml @@ -1,24 +1,6 @@ - - - - - - - - - - - - - - - - - - Test Westminster Constituency @@ -26,41 +8,6 @@ Test Constituency Value - - - - - - - 1 - Test Hansard Section - - - - - - - - - - - - - 2 - com.theyworkforyou/test/hansard/2 - 11 - 2014-01-01 - 10:00 - 1 - 0 - 1 - 0 - 1 - 2 - - - - 1 @@ -289,14 +236,6 @@ 0 - - - - - - - - 363 @@ -931,21 +870,5 @@ Majority - - - - - - - - - - - - - - - - diff --git a/www/docs/mp/index.php b/www/docs/mp/index.php index 3056ab2489..0682b51cf5 100644 --- a/www/docs/mp/index.php +++ b/www/docs/mp/index.php @@ -1157,7 +1157,7 @@ function person_party_policy_diffs($MEMBER, $policiesList, $only_diffs) { $policySummaries = $divisions->getMemberDivisionDetails(); $party = new MySociety\TheyWorkForYou\Party($MEMBER->party()); - $partyCohort = new MySociety\TheyWorkForYou\PartyCohort($MEMBER->cohortKey()); + $partyCohort = new MySociety\TheyWorkForYou\PartyCohort($MEMBER->person_id(), $MEMBER->cohortParty()); $data['party_positions'] = $partyCohort->getAllPolicyPositions($policiesList); # house hard coded as this is only used for the party position # comparison which is Commons only