reduce complexity of user driver license

5 files changed, 113 insertions, 92 deletions

diff --git a/includes/controller/user_driver_licenses_controller.php b/includes/controller/user_driver_licenses_controller.php
index 595fc3b6..818ff86b 100644
--- a/includes/controller/user_driver_licenses_controller.php
+++ b/includes/controller/user_driver_licenses_controller.php
@@ -33,11 +33,9 @@ function user_driver_licenses_controller() {
     redirect(page_link_to(''));
   }
   
-  if (! isset($_REQUEST['action'])) {
-    $_REQUEST['action'] = 'edit';
-  }
+  $action = strip_request_item('action', 'edit');
   
-  switch ($_REQUEST['action']) {
+  switch ($action) {
     default:
     case 'edit':
       return user_driver_license_edit_controller();
@@ -87,64 +85,48 @@ function user_driver_license_edit_controller() {
     redirect(user_driver_license_edit_link());
   }
   
-  $wants_to_drive = false;
-  $has_car = false;
-  $has_license_car = false;
-  $has_license_3_5t_transporter = false;
-  $has_license_7_5t_truck = false;
-  $has_license_12_5t_truck = false;
-  $has_license_forklift = false;
-  
   $user_driver_license = UserDriverLicense($user_source['UID']);
-  if ($user_driver_license != null) {
+  if ($user_driver_license == null) {
+    $wants_to_drive = false;
+    $user_driver_license = UserDriverLicense_new($user_source);
+  } else {
     $wants_to_drive = true;
-    $has_car = $user_driver_license['has_car'];
-    $has_license_car = $user_driver_license['has_license_car'];
-    $has_license_3_5t_transporter = $user_driver_license['has_license_3_5t_transporter'];
-    $has_license_7_5t_truck = $user_driver_license['has_license_7_5t_truck'];
-    $has_license_12_5t_truck = $user_driver_license['has_license_12_5t_truck'];
-    $has_license_forklift = $user_driver_license['has_license_forklift'];
   }
   
   if (isset($_REQUEST['submit'])) {
     $valid = true;
     $wants_to_drive = isset($_REQUEST['wants_to_drive']);
-    $has_car = isset($_REQUEST['has_car']);
-    $has_license_car = isset($_REQUEST['has_license_car']);
-    $has_license_3_5t_transporter = isset($_REQUEST['has_license_3_5t_transporter']);
-    $has_license_7_5t_truck = isset($_REQUEST['has_license_7_5t_truck']);
-    $has_license_12_5t_truck = isset($_REQUEST['has_license_12_5t_truck']);
-    $has_license_forklift = isset($_REQUEST['has_license_forklift']);
-    
-    if ($wants_to_drive && ! $has_license_car && ! $has_license_3_5t_transporter && ! $has_license_7_5t_truck && ! $has_license_12_5t_truck && ! $has_license_forklift) {
-      $valid = false;
-      error(_("Please select at least one driving license."));
-    }
-    
-    if ($valid) {
-      if (! $wants_to_drive && $user_driver_license != null) {
-        UserDriverLicenses_delete($user_source['UID']);
-        engelsystem_log("Driver license information removed.");
-        success(_("Your driver license information has been removed."));
-      } else {
-        if ($wants_to_drive) {
-          if ($user_driver_license == null) {
-            $result = UserDriverLicenses_create($user_source['UID'], $has_car, $has_license_car, $has_license_3_5t_transporter, $has_license_7_5t_truck, $has_license_12_5t_truck, $has_license_forklift);
-          } else {
-            $result = UserDriverLicenses_update($user_source['UID'], $has_car, $has_license_car, $has_license_3_5t_transporter, $has_license_7_5t_truck, $has_license_12_5t_truck, $has_license_forklift);
-          }
-          engelsystem_log("Driver license information updated.");
+    if ($wants_to_drive) {
+      $user_driver_license['has_car'] = isset($_REQUEST['has_car']);
+      $user_driver_license['has_license_car'] = isset($_REQUEST['has_license_car']);
+      $user_driver_license['has_license_3_5t_transporter'] = isset($_REQUEST['has_license_3_5t_transporter']);
+      $user_driver_license['has_license_7_5t_truck'] = isset($_REQUEST['has_license_7_5t_truck']);
+      $user_driver_license['has_license_12_5t_truck'] = isset($_REQUEST['has_license_12_5t_truck']);
+      $user_driver_license['has_license_forklift'] = isset($_REQUEST['has_license_forklift']);
+      
+      if (UserDriverLicense_valid($user_driver_license)) {
+        if ($user_driver_license == null) {
+          UserDriverLicenses_create($user_driver_license);
+        } else {
+          UserDriverLicenses_update($user_driver_license);
         }
+        engelsystem_log("Driver license information updated.");
         success(_("Your driver license information has been saved."));
+        redirect(user_link($user_source));
+      } else {
+        error(_("Please select at least one driving license."));
       }
-      
+    } elseif ($user_driver_license['id'] != null) {
+      UserDriverLicenses_delete($user_source['UID']);
+      engelsystem_log("Driver license information removed.");
+      success(_("Your driver license information has been removed."));
       redirect(user_link($user_source));
     }
   }
   
   return [
       sprintf(_("Edit %s driving license information"), $user_source['Nick']),
-      UserDriverLicense_edit_view($user_source, $wants_to_drive, $has_car, $has_license_car, $has_license_3_5t_transporter, $has_license_7_5t_truck, $has_license_12_5t_truck, $has_license_forklift) 
+      UserDriverLicense_edit_view($user_source, $wants_to_drive, $user_driver_license) 
   ];
 }
 
diff --git a/includes/model/ShiftEntry_model.php b/includes/model/ShiftEntry_model.php
index 63127bc7..99f67028 100644
--- a/includes/model/ShiftEntry_model.php
+++ b/includes/model/ShiftEntry_model.php
@@ -1,6 +1,22 @@
 <?php
 
 /**
+ * Returns an array with the attributes of shift entries.
+ * FIXME! Needs entity object.
+ */
+function ShiftEntry_new() {
+  return [
+      'id' => null,
+      'SID' => null,
+      'TID' => null,
+      'UID' => null,
+      'Comment' => null,
+      'freeloaded_comment' => null,
+      'freeloaded' => false 
+  ];
+}
+
+/**
  * Counts all freeloaded shifts.
  */
 function ShiftEntries_freeleaded_count() {
diff --git a/includes/model/UserDriverLicenses_model.php b/includes/model/UserDriverLicenses_model.php
index 32785c92..6a3f14ab 100644
--- a/includes/model/UserDriverLicenses_model.php
+++ b/includes/model/UserDriverLicenses_model.php
@@ -1,8 +1,37 @@
 <?php
 
 /**
+ * Returns a new empty UserDriverLicense
+ * FIXME entity object needed
+ */
+function UserDriverLicense_new($user) {
+  return [
+      'user_id' => $user['UID'],
+      'has_car' => false,
+      'has_license_car' => false,
+      'has_license_3_5t_transporter' => false,
+      'has_license_7_5t_truck' => false,
+      'has_license_12_5t_truck' => false,
+      'has_license_forklift' => false 
+  ];
+}
+
+/**
+ * Is it valid?
+ *
+ * @param UserDriverLicense $user_driver_license
+ *          The UserDriverLicense to check
+ * @return boolean
+ */
+function UserDriverLicense_valid($user_driver_license) {
+  return $user_driver_license['has_car'] || $user_driver_license['has_license_car'] || $user_driver_license['has_license_3_5t_transporter'] || $user_driver_license['has_license_7_5t_truck'] || $user_driver_license['has_license_12_5t_truck'] || $user_driver_license['has_license_forklift'];
+}
+
+/**
  * Get a users driver license information
- * @param int $user_id The users id
+ *
+ * @param int $user_id
+ *          The users id
  */
 function UserDriverLicense($user_id) {
   $user_driver_license = sql_select("SELECT * FROM `UserDriverLicenses` WHERE `user_id`='" . sql_escape($user_id) . "'");
@@ -19,24 +48,19 @@ function UserDriverLicense($user_id) {
 /**
  * Create a user's driver license entry
  *
- * @param bool $user_id          
- * @param bool $has_car          
- * @param bool $has_license_car          
- * @param bool $has_license_3_5t_transporter          
- * @param bool $has_license_7_5t_truck          
- * @param bool $has_license_12_5t_truck          
- * @param bool $has_license_forklift          
+ * @param UserDriverLicense $user_driver_license
+ *          The UserDriverLicense to create
  */
-function UserDriverLicenses_create($user_id, $has_car, $has_license_car, $has_license_3_5t_transporter, $has_license_7_5t_truck, $has_license_12_5t_truck, $has_license_forklift) {
+function UserDriverLicenses_create($user_driver_license) {
   $result = sql_query("
       INSERT INTO `UserDriverLicenses` SET
-      `user_id`=" . sql_escape($user_id) . ",
-      `has_car`=" . sql_bool($has_car) . ",
-      `has_license_car`=" . sql_bool($has_license_car) . ",
-      `has_license_3_5t_transporter`=" . sql_bool($has_license_3_5t_transporter) . ",
-      `has_license_7_5t_truck`=" . sql_bool($has_license_7_5t_truck) . ",
-      `has_license_12_5t_truck`=" . sql_bool($has_license_12_5t_truck) . ",
-      `has_license_forklift`=" . sql_bool($has_license_forklift));
+      `user_id`=" . sql_escape($user_driver_license['user_id']) . ",
+      `has_car`=" . sql_bool($user_driver_license['has_car']) . ",
+      `has_license_car`=" . sql_bool($user_driver_license['has_license_car']) . ",
+      `has_license_3_5t_transporter`=" . sql_bool($user_driver_license['has_license_3_5t_transporter']) . ",
+      `has_license_7_5t_truck`=" . sql_bool($user_driver_license['has_license_7_5t_truck']) . ",
+      `has_license_12_5t_truck`=" . sql_bool($user_driver_license['has_license_12_5t_truck']) . ",
+      `has_license_forklift`=" . sql_bool($user_driver_license['has_license_forklift']));
   if ($result === false) {
     engelsystem_error('Unable to create user driver license');
   }
@@ -46,23 +70,18 @@ function UserDriverLicenses_create($user_id, $has_car, $has_license_car, $has_li
 /**
  * Update a user's driver license entry
  *
- * @param bool $user_id          
- * @param bool $has_car          
- * @param bool $has_license_car          
- * @param bool $has_license_3_5t_transporter          
- * @param bool $has_license_7_5t_truck          
- * @param bool $has_license_12_5t_truck          
- * @param bool $has_license_forklift          
+ * @param UserDriverLicense $user_driver_license
+ *          The UserDriverLicense to update
  */
-function UserDriverLicenses_update($user_id, $has_car, $has_license_car, $has_license_3_5t_transporter, $has_license_7_5t_truck, $has_license_12_5t_truck, $has_license_forklift) {
+function UserDriverLicenses_update($user_driver_license) {
   $result = sql_query("UPDATE `UserDriverLicenses` SET
-      `has_car`=" . sql_bool($has_car) . ",
-      `has_license_car`=" . sql_bool($has_license_car) . ",
-      `has_license_3_5t_transporter`=" . sql_bool($has_license_3_5t_transporter) . ",
-      `has_license_7_5t_truck`=" . sql_bool($has_license_7_5t_truck) . ",
-      `has_license_12_5t_truck`=" . sql_bool($has_license_12_5t_truck) . ",
-      `has_license_forklift`=" . sql_bool($has_license_forklift) . "
-      WHERE `user_id`='" . sql_escape($user_id) . "'");
+      `has_car`=" . sql_bool($user_driver_license['has_car']) . ",
+      `has_license_car`=" . sql_bool($user_driver_license['has_license_car']) . ",
+      `has_license_3_5t_transporter`=" . sql_bool($user_driver_license['has_license_3_5t_transporter']) . ",
+      `has_license_7_5t_truck`=" . sql_bool($user_driver_license['has_license_7_5t_truck']) . ",
+      `has_license_12_5t_truck`=" . sql_bool($user_driver_license['has_license_12_5t_truck']) . ",
+      `has_license_forklift`=" . sql_bool($user_driver_license['has_license_forklift']) . "
+      WHERE `user_id`='" . sql_escape($user_driver_license['user_id']) . "'");
   if ($result === false) {
     engelsystem_error("Unable to update user driver license information");
   }
diff --git a/includes/pages/user_shifts.php b/includes/pages/user_shifts.php
index 206a1586..5703ea8b 100644
--- a/includes/pages/user_shifts.php
+++ b/includes/pages/user_shifts.php
@@ -8,6 +8,10 @@ function shifts_title() {
 
 /**
  * Start different controllers for deleting shifts and shift_entries, edit shifts and add shift entries.
+ * FIXME:
+ * Transform into shift controller and shift entry controller.
+ * Split actions into shift edit, shift delete, shift entry edit, shift entry delete
+ * Introduce simpler and beautiful actions for shift entry join/leave for users
  */
 function user_shifts() {
   global $user;
diff --git a/includes/view/UserDriverLicenses_view.php b/includes/view/UserDriverLicenses_view.php
index c07e5e1f..5a3effeb 100644
--- a/includes/view/UserDriverLicenses_view.php
+++ b/includes/view/UserDriverLicenses_view.php
@@ -2,16 +2,16 @@
 
 /**
  * Edit a user's driving license information.
+ *
  * @param User $user_source
+ *          The user
  * @param bool $wants_to_drive
- * @param bool $has_car
- * @param bool $has_license_car
- * @param bool $has_license_3_5t_transporter
- * @param bool $has_license_7_5t_truck
- * @param bool $has_license_12_5t_truck
- * @param bool $has_license_forklift
+ *          true, if the user wants to drive
+ * @param UserDriverLicense $user_driver_license
+ *          The user driver license
+ * @return string
  */
-function UserDriverLicense_edit_view($user_source, $wants_to_drive, $has_car, $has_license_car, $has_license_3_5t_transporter, $has_license_7_5t_truck, $has_license_12_5t_truck, $has_license_forklift) {
+function UserDriverLicense_edit_view($user_source, $wants_to_drive, $user_driver_license) {
   return page_with_title(sprintf(_("Edit %s driving license information"), User_Nick_render($user_source)), [
       buttons([
           button(user_link($user_source), _("Back to profile"), 'back') 
@@ -22,17 +22,17 @@ function UserDriverLicense_edit_view($user_source, $wants_to_drive, $has_car, $h
           form_checkbox('wants_to_drive', _("I am willing to operate cars for the PL"), $wants_to_drive),
           div('panel panel-default', [
               div('panel-body', [
-                  form_checkbox('has_car', _("I have my own car with me and am willing to use it for the PL (You'll get reimbursed for fuel)"), $has_car),
+                  form_checkbox('has_car', _("I have my own car with me and am willing to use it for the PL (You'll get reimbursed for fuel)"), $user_driver_license['has_car']),
                   heading(_("Driver license"), 3),
-                  form_checkbox('has_license_car', _("Car"), $has_license_car),
-                  form_checkbox('has_license_3_5t_transporter', _("Transporter 3,5t"), $has_license_3_5t_transporter),
-                  form_checkbox('has_license_7_5t_truck', _("Truck 7,5t"), $has_license_7_5t_truck),
-                  form_checkbox('has_license_12_5t_truck', _("Truck 12,5t"), $has_license_12_5t_truck),
-                  form_checkbox('has_license_forklift', _("Forklift"), $has_license_forklift) 
+                  form_checkbox('has_license_car', _("Car"), $user_driver_license['has_license_car']),
+                  form_checkbox('has_license_3_5t_transporter', _("Transporter 3,5t"), $user_driver_license['has_license_3_5t_transporter']),
+                  form_checkbox('has_license_7_5t_truck', _("Truck 7,5t"), $user_driver_license['has_license_7_5t_truck']),
+                  form_checkbox('has_license_12_5t_truck', _("Truck 12,5t"), $user_driver_license['has_license_12_5t_truck']),
+                  form_checkbox('has_license_forklift', _("Forklift"), $user_driver_license['has_license_forklift']) 
               ]) 
           ], 'driving_license'),
           form_submit('submit', _("Save")) 
-      ]) ,
+      ]),
       '<script type="text/javascript">
         $(function() {
           if($("#wants_to_drive").is(":checked"))
@@ -49,7 +49,7 @@ function UserDriverLicense_edit_view($user_source, $wants_to_drive, $has_car, $h
             }
           );
         });
-      </script>'
+      </script>' 
   ]);
 }