From 889a8b8043138a53a5a2754029e77e2a90ceb573 Mon Sep 17 00:00:00 2001
From: Patrick Dawkins <pjcdawkins@googlemail.com>
Date: Wed, 14 Sep 2011 15:14:14 +0100
Subject: [PATCH] Cleaned up some comments, form settings, validation bugs,
 etc.

---
 election-candidate.pages.inc | 17 ------------
 election-post.admin.inc      | 33 ++++++++++++-----------
 election.admin.inc           | 10 +++----
 election.module              | 51 ++++++++++++++++++------------------
 views/election.views.inc     |  6 +++--
 5 files changed, 53 insertions(+), 64 deletions(-)

diff --git a/election-candidate.pages.inc b/election-candidate.pages.inc
index ca65f7c..dbc9b87 100644
--- a/election-candidate.pages.inc
+++ b/election-candidate.pages.inc
@@ -68,23 +68,6 @@ function election_nomination_form($form, &$form_state, $election, $candidate) {
     '#required' => TRUE,
   );
 
-/*
-  $form['first_name'] = array(
-    '#type' => 'textfield',
-    '#title' => t('First name'),
-    '#default_value' => isset($candidate->first_name)? $candidate->first_name : NULL,
-    '#size' => 20,
-    '#required' => TRUE,
-  );
-  $form['last_name'] = array(
-    '#type' => 'textfield',
-    '#title' => t('Last name'),
-    '#default_value' => isset($candidate->first_name)? $candidate->first_name : NULL,
-    '#size' => 20,
-    '#required' => TRUE,
-  );
- */
-
   $form['mail'] = array(
     '#type' => 'textfield',
     '#title' => t('Email address'),
diff --git a/election-post.admin.inc b/election-post.admin.inc
index faa9dac..399f79c 100644
--- a/election-post.admin.inc
+++ b/election-post.admin.inc
@@ -50,11 +50,6 @@ function election_post_form($form, &$form_state, $election, $post) {
     '#required' => TRUE,
   );
 
-  $form['vtabs'] = array(
-    '#type' => 'vertical_tabs',
-    '#weight' => 99,
-  );
-
   $type = _election_type_load($election->type);
   $post_key = $type['post key'];
   $post_form_function = 'election_post_' . $post_key . '_form';
@@ -62,11 +57,14 @@ function election_post_form($form, &$form_state, $election, $post) {
     $form = $post_form_function($form, $form_state, $election, $post);
   }
 
-  $form['vtabs']['voting'] = array(
+  $form['voting'] = array(
     '#type' => 'fieldset',
     '#title' => t('Voting settings'),
+    '#collapsible' => TRUE,
+    '#collapsed' => TRUE,
+    '#weight' => 98,
   );
-  $form['vtabs']['voting']['vstatus_inheritance'] = array(
+  $form['voting']['vstatus_inheritance'] = array(
     '#type' => 'radios',
     '#title' => t('Voting status'),
     '#default_value' => isset($post->vstatus_inheritance)? $post->vstatus_inheritance : ELECTION_POST_STATUS_INHERIT,
@@ -83,12 +81,14 @@ function election_post_form($form, &$form_state, $election, $post) {
       $post->electorates[$row['electorate_id']] = 1;
     }
   }
-
-  $form['vtabs']['conditions'] = array(
+  $form['conditions'] = array(
     '#type' => 'fieldset',
     '#title' => t('Conditional voting'),
+    '#collapsible' => TRUE,
+    '#collapsed' => TRUE,
+    '#weight' => 99,
   );
-  $form['vtabs']['conditions']['help'] = array(
+  $form['conditions']['help'] = array(
     'title' => array(
       '#prefix' => '<div class="form-item"><label for="edit-electorates">',
       '#markup' => t('Assign electorates'),
@@ -96,7 +96,7 @@ function election_post_form($form, &$form_state, $election, $post) {
     ),
     'help' => array(
       '#prefix' => '<p class="voting-conditions-help">',
-      '#markup' => t('To be part of an electorate, the user must match ALL of that electorate\'s conditions. If the user is part of ANY of the electorates assigned below, AND has the permission "vote in elections", then he/she may cast a vote for this @post_type_name.', array('@post_type_name' => _election_get_posts_name($election->type))),
+      '#markup' => t('A user may cast a vote for this @post_type_name if he/she is a member of ANY of the electorates assigned here.', array('@post_type_name' => _election_get_posts_name($election->type))),
       '#suffix' => '</p></div>',
     ),
   );
@@ -129,7 +129,7 @@ function election_post_form($form, &$form_state, $election, $post) {
       'description' => $electorate['description'],
     );
   }
-  $form['vtabs']['conditions']['electorates'] = array(
+  $form['conditions']['electorates'] = array(
     '#type' => 'tableselect',
     '#attributes' => array('class', 'select-electorates'),
     '#default_value' => isset($post->electorates)? (array) $post->electorates : array(),
@@ -238,12 +238,15 @@ function election_post_position_form($form, &$form_state, $election, $post) {
     ),
   );
 
-  $form['vtabs']['nominations'] = array(
+  $form['nominations'] = array(
     '#type' => 'fieldset',
     '#title' => t('Nominations settings'),
+    '#collapsible' => TRUE,
+    '#collapsed' => TRUE,
+    '#weight' => 97,
   );
 
-  $form['vtabs']['nominations']['nstatus_inheritance'] = array(
+  $form['nominations']['nstatus_inheritance'] = array(
     '#type' => 'radios',
     '#title' => t('Nominations status'),
     '#default_value' => isset($post->nstatus_inheritance)? $post->nstatus_inheritance : ELECTION_POST_STATUS_INHERIT,
@@ -266,7 +269,7 @@ function election_post_position_form($form, &$form_state, $election, $post) {
   if (!empty($post->require_photo)) {
     $nominations_required_fields_default[] = 'photo';
   }
-  $form['vtabs']['nominations']['nominations_required_fields'] = array(
+  $form['nominations']['nominations_required_fields'] = array(
     '#type' => 'checkboxes',
     '#title' => t('Nomination form required fields'),
     '#default_value' => $nominations_required_fields_default,
diff --git a/election.admin.inc b/election.admin.inc
index 29e39cd..e706d6f 100644
--- a/election.admin.inc
+++ b/election.admin.inc
@@ -385,6 +385,11 @@ function election_form_validate($form, &$form_state) {
           && strtotime($form_state['values']['nclose_time']) <= strtotime($form_state['values']['nopen_time'])) {
       form_set_error('nschedule', t('The closing time of nominations must be after the opening time.'));
     }
+    // Check that the voting time is after the nominations close.
+    elseif (@$form_state['values']['vstatus'] == ELECTION_STATUS_SCHEDULED
+          && strtotime($form_state['values']['vopen_time']) < strtotime($form_state['values']['nclose_time'])) {
+      form_set_error('nschedule', t('Nominations can only be scheduled so that they close before the start of voting.'));
+    }
   }
 
   // VALIDATE VOTING SCHEDULE
@@ -399,11 +404,6 @@ function election_form_validate($form, &$form_state) {
           && strtotime($form_state['values']['vclose_time']) <= strtotime($form_state['values']['vopen_time'])) {
       form_set_error('vschedule', t('The closing time of voting must be after the opening time.'));
     }
-    // Check that the voting time is after the nominations close.
-    elseif ($form_state['values']['vstatus'] == ELECTION_STATUS_SCHEDULED
-          && strtotime($form_state['values']['vopen_time']) < strtotime($form_state['values']['nclose_time'])) {
-      form_set_error('vschedule', t('Voting can only be scheduled to open after the close of nominations.'));
-    }
   }
 
   // Check other Fields.
diff --git a/election.module b/election.module
index a3538a3..e7198dc 100644
--- a/election.module
+++ b/election.module
@@ -377,8 +377,8 @@ function election_modules_disabled($modules = array()) {
 }
 
 /*
- * Delete electorates from the DB only if they don't exist in
- * election_post_electorate AND they don't exist in code.
+ * Delete electorates from the DB only if they are not assigned to any posts AND
+ * they do not (or no longer) exist in code.
  *
  * @return void
  */
@@ -388,9 +388,12 @@ function _election_uninstall_code_electorates() {
   try {
     $db_electorates = db_query('SELECT electorate_id, machine_name FROM {election_electorate} WHERE locked = 1');
     while ($db_electorate = $db_electorates->fetchAssoc()) {
+      // Don't delete electorates that (still) exist in code.
       if (isset($code_electorates[$db_electorate['machine_name']])) {
         continue;
       }
+      // Don't delete assigned electorates.
+      // @todo with a bit of thought this could be run as a JOIN on the $db_electorates query above.
       $assigned = db_query_range(
         'SELECT 1 FROM {election_post_electorate} WHERE electorate_id = :eid',
         0, 1,
@@ -411,7 +414,12 @@ function _election_uninstall_code_electorates() {
   }
   catch (Exception $e) {
     $transaction->rollback();
-    watchdog_exception('election', $e, NULL, WATCHDOG_ERROR);
+    watchdog_exception(
+      'election',
+      $e,
+      'Failed to remove from the database those electorates that no longer exist in code.',
+      WATCHDOG_ERROR
+    );
     return FALSE;
   }
 }
@@ -426,26 +434,19 @@ function _election_uninstall_code_electorates() {
  * @return void
  */
 function _election_install_code_electorates() {
-  $transaction = db_transaction();
-  try {
-    foreach (_election_get_code_electorates() as $electorate_mn => $electorate) {
-      $record = array(
-        'machine_name' => $electorate_mn,
-        'name' => empty($electorate['name'])? $electorate_mn : $electorate['name'],
-        'description' => @$electorate['description'],
-        'locked' => 1,
-        'changed' => REQUEST_TIME,
-      );
-      db_merge('election_electorate')
-        ->key(array('machine_name' => $electorate_mn))
-        ->updateFields($record)
-        ->insertFields($record + array('created' => REQUEST_TIME))
-        ->execute();
-    }
-  }
-  catch (Exception $e) {
-    $transaction->rollback();
-    watchdog_exception('election', $e, NULL, WATCHDOG_ERROR);
+  foreach (_election_get_code_electorates() as $electorate_mn => $electorate) {
+    $record = array(
+      'machine_name' => $electorate_mn,
+      'name' => empty($electorate['name'])? $electorate_mn : $electorate['name'],
+      'description' => @$electorate['description'],
+      'locked' => 1,
+      'changed' => REQUEST_TIME,
+    );
+    db_merge('election_electorate')
+      ->key(array('machine_name' => $electorate_mn))
+      ->updateFields($record)
+      ->insertFields($record + array('created' => REQUEST_TIME))
+      ->execute();
   }
 }
 
@@ -641,7 +642,7 @@ function election_menu() {
 
   $items['election'] = array(
     'title' => 'Elections',
-    'page callback' => 'drupal_get_form',
+    'page callback' => 'drupal_get_form', // @todo make this views_embed_view or a wrapper
     'page arguments' => array('election_list_form'),
     'access callback' => 'election_access',
     'access arguments' => array('view'),
@@ -1212,7 +1213,7 @@ function _election_is_open($status, $open_time, $close_time) {
 function _election_format_status(stdClass $election, $status_prefix = 'v') {
   // Nominations aren't relevant to referendums.
   // @todo make this extensible
-  if ($election->type == 'referendum' && $status_prefix == 'n') {
+  if (@$election->type == 'referendum' && $status_prefix == 'n') {
     return t('N/A');
   }
   $status = $election->{$status_prefix . 'status'};
diff --git a/views/election.views.inc b/views/election.views.inc
index b71e720..3a5b9ee 100644
--- a/views/election.views.inc
+++ b/views/election.views.inc
@@ -2,8 +2,10 @@
 /**
  * @file
  * Views integration for the Election module.
- *
- * @todo this is totally unfinished and doesn't work at all.
+ */
+
+/**
+ * Implements hook_views_data_alter().
  */
 function election_views_data_alter(&$data) {
 
-- 
GitLab