reduce complexity of angeltype edit controller

5 files changed, 82 insertions, 76 deletions

diff --git a/includes/controller/angeltypes_controller.php b/includes/controller/angeltypes_controller.php
index 548cf3e7..86c3cd02 100644
--- a/includes/controller/angeltypes_controller.php
+++ b/includes/controller/angeltypes_controller.php
@@ -73,12 +73,7 @@ function angeltype_delete_controller() {
   }
   
   if (isset($_REQUEST['confirmed'])) {
-    $result = AngelType_delete($angeltype);
-    if ($result === false) {
-      engelsystem_error("Unable to delete angeltype.");
-    }
-    
-    engelsystem_log("Deleted angeltype: " . AngelType_name_render($angeltype));
+    AngelType_delete($angeltype);
     success(sprintf(_("Angeltype %s deleted."), AngelType_name_render($angeltype)));
     redirect(page_link_to('angeltypes'));
   }
@@ -95,11 +90,8 @@ function angeltype_delete_controller() {
 function angeltype_edit_controller() {
   global $privileges, $user;
   
-  $angeltype = null;
-  $name = "";
-  $restricted = false;
-  $description = "";
-  $requires_driver_license = false;
+  // In supporter mode only allow to modify description
+  $supporter_mode = ! in_array('admin_angel_types', $privileges);
   
   if (isset($_REQUEST['angeltype_id'])) {
     $angeltype = AngelType($_REQUEST['angeltype_id']);
@@ -107,68 +99,50 @@ function angeltype_edit_controller() {
       redirect(page_link_to('angeltypes'));
     }
     
-    $name = $angeltype['name'];
-    $restricted = $angeltype['restricted'];
-    $description = $angeltype['description'];
-    $requires_driver_license = $angeltype['requires_driver_license'];
-    
     if (! User_is_AngelType_supporter($user, $angeltype)) {
       redirect(page_link_to('angeltypes'));
     }
   } else {
-    if (! in_array('admin_angel_types', $privileges)) {
+    if ($supporter_mode) {
       redirect(page_link_to('angeltypes'));
     }
+    $angeltype = AngelType_new();
   }
   
-  // In supporter mode only allow to modify description
-  $supporter_mode = ! in_array('admin_angel_types', $privileges);
-  
   if (isset($_REQUEST['submit'])) {
     $valid = true;
     
     if (! $supporter_mode) {
       if (isset($_REQUEST['name'])) {
         $result = AngelType_validate_name($_REQUEST['name'], $angeltype);
-        $name = $result->getValue();
+        $angeltype['name'] = $result->getValue();
         if (! $result->isValid()) {
           $valid = false;
           error(_("Please check the name. Maybe it already exists."));
         }
       }
       
-      $restricted = isset($_REQUEST['restricted']);
-      $requires_driver_license = isset($_REQUEST['requires_driver_license']);
+      $angeltype['restricted'] = isset($_REQUEST['restricted']);
+      $angeltype['requires_driver_license'] = isset($_REQUEST['requires_driver_license']);
     }
     
-    if (isset($_REQUEST['description'])) {
-      $description = strip_request_item_nl('description');
-    }
+    $angeltype['description'] = strip_request_item_nl('description', $angeltype['description']);
     
     if ($valid) {
-      if (isset($angeltype)) {
-        $result = AngelType_update($angeltype['id'], $name, $restricted, $description, $requires_driver_license);
-        if ($result === false) {
-          engelsystem_error("Unable to update angeltype.");
-        }
-        engelsystem_log("Updated angeltype: " . $name . ($restricted ? ", restricted" : "") . ($requires_driver_license ? ", requires driver license" : ""));
-        $angeltype_id = $angeltype['id'];
+      if ($angeltype['id'] != null) {
+        AngelType_update($angeltype);
       } else {
-        $angeltype_id = AngelType_create($name, $restricted, $description, $requires_driver_license);
-        if ($angeltype_id === false) {
-          engelsystem_error("Unable to create angeltype.");
-        }
-        engelsystem_log("Created angeltype: " . $name . ($restricted ? ", restricted" : "") . ($requires_driver_license ? ", requires driver license" : ""));
+        $angeltype = AngelType_create($angeltype);
       }
       
       success("Angel type saved.");
-      redirect(angeltype_link($angeltype_id));
+      redirect(angeltype_link($angeltype['id']));
     }
   }
   
   return [
-      sprintf(_("Edit %s"), $name),
-      AngelType_edit_view($name, $restricted, $description, $supporter_mode, $requires_driver_license) 
+      sprintf(_("Edit %s"), $angeltype['name']),
+      AngelType_edit_view($angeltype, $supporter_mode) 
   ];
 }
 
diff --git a/includes/model/AngelType_model.php b/includes/model/AngelType_model.php
index fa81349d..86d64504 100644
--- a/includes/model/AngelType_model.php
+++ b/includes/model/AngelType_model.php
@@ -1,56 +1,77 @@
 <?php
 
 /**
+ * Returns an array containing the basic attributes of angeltypes.
+ * FIXME! This is the big sign for needing entity objects
+ */
+function AngelType_new() {
+  return [
+      'id' => null,
+      'name' => "",
+      'restricted' => false,
+      'description' => '',
+      'requires_driver_license' => false 
+  ];
+}
+
+/**
  * Delete an Angeltype.
- * 
+ *
  * @param Angeltype $angeltype          
  */
 function AngelType_delete($angeltype) {
-  return sql_query("
+  $result = sql_query("
       DELETE FROM `AngelTypes` 
       WHERE `id`='" . sql_escape($angeltype['id']) . "' 
       LIMIT 1");
+  if ($result === false) {
+    engelsystem_error("Unable to delete angeltype.");
+  }
+  engelsystem_log("Deleted angeltype: " . AngelType_name_render($angeltype));
+  return $result;
 }
 
 /**
  * Update Angeltype.
  *
- * @param int $angeltype_id          
- * @param string $name          
- * @param bool $restricted          
- * @param string $description          
- * @param bool $requires_driver_license          
+ * @param Angeltype $angeltype
+ *          The angeltype
  */
-function AngelType_update($angeltype_id, $name, $restricted, $description, $requires_driver_license) {
-  return sql_query("
+function AngelType_update($angeltype) {
+  $result = sql_query("
       UPDATE `AngelTypes` SET 
-      `name`='" . sql_escape($name) . "', 
-      `restricted`=" . sql_bool($restricted) . ",
-      `description`='" . sql_escape($description) . "',
-      `requires_driver_license`=" . sql_bool($requires_driver_license) . "
-      WHERE `id`='" . sql_escape($angeltype_id) . "' 
-      LIMIT 1");
+      `name`='" . sql_escape($angeltype['name']) . "', 
+      `restricted`=" . sql_bool($angeltype['restricted']) . ",
+      `description`='" . sql_escape($angeltype['description']) . "',
+      `requires_driver_license`=" . sql_bool($angeltype['requires_driver_license']) . "
+      WHERE `id`='" . sql_escape($angeltype['id']) . "'");
+  if ($result === false) {
+    engelsystem_error("Unable to update angeltype.");
+  }
+  engelsystem_log("Updated angeltype: " . $angeltype['name'] . ($angeltype['restricted'] ? ", restricted" : "") . ($angeltype['requires_driver_license'] ? ", requires driver license" : ""));
+  return $result;
 }
 
 /**
  * Create an Angeltype.
  *
- * @param string $name          
- * @param boolean $restricted          
- * @param string $description          
- * @return New Angeltype id
+ * @param Angeltype $angeltype
+ *          The angeltype
+ * @return the created angeltype
  */
-function AngelType_create($name, $restricted, $description, $requires_driver_license) {
+function AngelType_create($angeltype) {
   $result = sql_query("
       INSERT INTO `AngelTypes` SET 
-      `name`='" . sql_escape($name) . "', 
-      `restricted`=" . sql_bool($restricted) . ",
-      `description`='" . sql_escape($description) . "',
-      `requires_driver_license`=" . sql_bool($requires_driver_license));
+      `name`='" . sql_escape($angeltype['name']) . "', 
+      `restricted`=" . sql_bool($angeltype['restricted']) . ",
+      `description`='" . sql_escape($angeltype['description']) . "',
+      `requires_driver_license`=" . sql_bool($angeltype['requires_driver_license']));
   if ($result === false) {
-    return false;
+    engelsystem_error("Unable to create angeltype.");
   }
-  return sql_id();
+  $angeltype['id'] = sql_id();
+  engelsystem_log("Created angeltype: " . $angeltype['name'] . ($angeltype['restricted'] ? ", restricted" : "") . ($angeltype['requires_driver_license'] ? ", requires driver license" : ""));
+  return $angeltype;
 }
 
 /**
diff --git a/includes/pages/admin_log.php b/includes/pages/admin_log.php
index d45a232b..97b81585 100644
--- a/includes/pages/admin_log.php
+++ b/includes/pages/admin_log.php
@@ -9,7 +9,7 @@ function admin_log() {
   if (isset($_REQUEST['keyword'])) {
     $filter = strip_request_item('keyword');
   }
-  $log_entries_source = LogEntries_filter($_POST['keyword']);
+  $log_entries_source = LogEntries_filter($filter);
   
   $log_entries = [];
   foreach ($log_entries_source as $log_entry) {
diff --git a/includes/sys_page.php b/includes/sys_page.php
index 102be926..ad4d15de 100644
--- a/includes/sys_page.php
+++ b/includes/sys_page.php
@@ -166,8 +166,11 @@ function test_request_int($name) {
 /**
  * Gibt den gefilterten REQUEST Wert mit Zeilenumbrüchen zurück
  */
-function strip_request_item_nl($name) {
-  return preg_replace("/([^\p{L}\p{S}\p{P}\p{Z}\p{N}+\n]{1,})/ui", '', strip_tags($_REQUEST[$name]));
+function strip_request_item_nl($name, $default_value = null) {
+  if (isset($_REQUEST[$name])) {
+    return preg_replace("/([^\p{L}\p{S}\p{P}\p{Z}\p{N}+\n]{1,})/ui", '', strip_tags($_REQUEST[$name]));
+  }
+  return $default_value;
 }
 
 /**
diff --git a/includes/view/AngelTypes_view.php b/includes/view/AngelTypes_view.php
index 485ce384..a48f8d4c 100644
--- a/includes/view/AngelTypes_view.php
+++ b/includes/view/AngelTypes_view.php
@@ -47,18 +47,26 @@ function AngelType_delete_view($angeltype) {
   ]);
 }
 
-function AngelType_edit_view($name, $restricted, $description, $supporter_mode, $requires_driver_license) {
-  return page_with_title(sprintf(_("Edit %s"), $name), [
+/**
+ * Render angeltype edit form.
+ * 
+ * @param Angeltype $angeltype
+ *          The angeltype to edit
+ * @param boolean $supporter_mode
+ *          Is the user a supporter of this angeltype?
+ */
+function AngelType_edit_view($angeltype, $supporter_mode) {
+  return page_with_title(sprintf(_("Edit %s"), $angeltype['name']), [
       buttons([
           button(page_link_to('angeltypes'), _("Angeltypes"), 'back') 
       ]),
       msg(),
       form([
-          $supporter_mode ? form_info(_("Name"), $name) : form_text('name', _("Name"), $name),
-          $supporter_mode ? form_info(_("Restricted"), $restricted ? _("Yes") : _("No")) : form_checkbox('restricted', _("Restricted"), $restricted),
-          $supporter_mode ? form_info(_("Requires driver license"), $requires_driver_license ? _("Yes") : _("No")) : form_checkbox('requires_driver_license', _("Requires driver license"), $requires_driver_license),
+          $supporter_mode ? form_info(_("Name"), $angeltype['name']) : form_text('name', _("Name"), $angeltype['name']),
+          $supporter_mode ? form_info(_("Restricted"), $angeltype['restricted'] ? _("Yes") : _("No")) : form_checkbox('restricted', _("Restricted"), $angeltype['restricted']),
+          $supporter_mode ? form_info(_("Requires driver license"), $angeltype['requires_driver_license'] ? _("Yes") : _("No")) : form_checkbox('requires_driver_license', _("Requires driver license"), $angeltype['requires_driver_license']),
           form_info("", _("Restricted angel types can only be used by an angel if enabled by an archangel (double opt-in).")),
-          form_textarea('description', _("Description"), $description),
+          form_textarea('description', _("Description"), $angeltype['description']),
           form_info("", _("Please use markdown for the description.")),
           form_submit('submit', _("Save")) 
       ])