Merge pull request #167 from Icinga/FormRefactor

`ScheduleForm`, `TemplateForm`, `ReportForm`, and `TimeFrameForm` Refactoring
Icinga · Feb 2, 2023 · d3cdd00 · d3cdd00
2 parents 9134b7a + e6c921a
commit d3cdd00
Show file tree

Hide file tree

Showing 11 changed files with 140 additions and 122 deletions.
diff --git a/application/controllers/ReportController.php b/application/controllers/ReportController.php
@@ -67,12 +67,12 @@ public function editAction()
             $values[$name] = $value;
         }
 
-        $form = new ReportForm();
-        $form->setId($this->report->getId());
-        $form->populate($values);
-        $form->handleRequest(ServerRequest::fromGlobals());
-
-        $this->redirectForm($form, 'reporting/reports');
+        $form = ReportForm::fromId($this->report->getId())
+            ->on(ReportForm::ON_SUCCESS, function () {
+                $this->redirectNow('reporting/reports');
+            })
+            ->populate($values)
+            ->handleRequest(ServerRequest::fromGlobals());
 
         $this->addContent($form);
     }
@@ -84,13 +84,13 @@ public function sendAction()
         Environment::raiseExecutionTime();
         Environment::raiseMemoryLimit();
 
-        $form = new SendForm();
-        $form
+        $form = (new SendForm())
             ->setReport($this->report)
+            ->on(SendForm::ON_SUCCESS, function () {
+                $this->redirectNow("reporting/report?id={$this->report->getId()}");
+            })
             ->handleRequest(ServerRequest::fromGlobals());
 
-        $this->redirectForm($form, "reporting/report?id={$this->report->getId()}");
-
         $this->addContent($form);
     }
 
@@ -99,13 +99,12 @@ public function scheduleAction()
         $this->assertPermission('reporting/schedules');
         $this->addTitleTab($this->translate('Schedule'));
 
-        $form = new ScheduleForm();
-        $form
-            ->setReport($this->report)
+        $form = ScheduleForm::fromReport($this->report)
+            ->on(ScheduleForm::ON_SUCCESS, function () {
+                $this->redirectNow("reporting/report?id={$this->report->getId()}");
+            })
             ->handleRequest(ServerRequest::fromGlobals());
 
-        $this->redirectForm($form, "reporting/report?id={$this->report->getId()}");
-
         $this->addContent($form);
     }
 

diff --git a/application/controllers/ReportsController.php b/application/controllers/ReportsController.php
@@ -95,10 +95,11 @@ public function newAction()
         $this->assertPermission('reporting/reports');
         $this->addTitleTab($this->translate('New Report'));
 
-        $form = new ReportForm();
-        $form->handleRequest(ServerRequest::fromGlobals());
-
-        $this->redirectForm($form, 'reporting/reports');
+        $form = (new ReportForm())
+            ->on(ReportForm::ON_SUCCESS, function () {
+                $this->redirectNow('reporting/reports');
+            })
+            ->handleRequest(ServerRequest::fromGlobals());
 
         $this->addContent($form);
     }

diff --git a/application/controllers/TemplateController.php b/application/controllers/TemplateController.php
@@ -57,12 +57,11 @@ public function editAction()
 
         $template->settings = json_decode($template->settings, true);
 
-        $form = (new TemplateForm())
-            ->setTemplate($template);
-
-        $form->handleRequest(ServerRequest::fromGlobals());
-
-        $this->redirectForm($form, 'reporting/templates');
+        $form = TemplateForm::fromTemplate($template)
+            ->on(TemplateForm::ON_SUCCESS, function () {
+                $this->redirectNow('reporting/templates');
+            })
+            ->handleRequest(ServerRequest::fromGlobals());
 
         $this->addContent($form);
     }

diff --git a/application/controllers/TemplatesController.php b/application/controllers/TemplatesController.php
@@ -96,11 +96,11 @@ public function newAction()
         $this->assertPermission('reporting/templates');
         $this->addTitleTab($this->translate('New Template'));
 
-        $form = new TemplateForm();
-
-        $form->handleRequest(ServerRequest::fromGlobals());
-
-        $this->redirectForm($form, 'reporting/templates');
+        $form = (new TemplateForm())
+            ->on(TemplateForm::ON_SUCCESS, function () {
+                $this->redirectNow('reporting/templates');
+            })
+            ->handleRequest(ServerRequest::fromGlobals());
 
         $this->addContent($form);
     }

diff --git a/application/controllers/TimeframeController.php b/application/controllers/TimeframeController.php
@@ -34,14 +34,12 @@ public function editAction()
         ];
 
 
-        $form = (new TimeframeForm())
-            ->setId($this->timeframe->getId());
-
-        $form->populate($values);
-
-        $form->handleRequest(ServerRequest::fromGlobals());
-
-        $this->redirectForm($form, 'reporting/timeframes');
+        $form = TimeframeForm::fromId($this->timeframe->getId())
+            ->on(TimeframeForm::ON_SUCCESS, function () {
+                $this->redirectNow('reporting/timeframes');
+            })
+            ->populate($values)
+            ->handleRequest(ServerRequest::fromGlobals());
 
         $this->addContent($form);
     }

diff --git a/application/controllers/TimeframesController.php b/application/controllers/TimeframesController.php
@@ -94,10 +94,11 @@ public function newAction()
         $this->assertPermission('reporting/timeframes');
         $this->addTitleTab($this->translate('New Timeframe'));
 
-        $form = new TimeframeForm();
-        $form->handleRequest(ServerRequest::fromGlobals());
-
-        $this->redirectForm($form, 'reporting/timeframes');
+        $form = (new TimeframeForm())
+            ->on(TimeframeForm::ON_SUCCESS, function () {
+                $this->redirectNow('reporting/timeframes');
+            })
+            ->handleRequest(ServerRequest::fromGlobals());
 
         $this->addContent($form);
     }

diff --git a/library/Reporting/Web/Controller.php b/library/Reporting/Web/Controller.php
@@ -4,19 +4,8 @@
 
 namespace Icinga\Module\Reporting\Web;
 
-use ipl\Html\Form;
 use ipl\Web\Compat\CompatController;
 
 class Controller extends CompatController
 {
-    protected function redirectForm(Form $form, $url)
-    {
-        if (
-            $form->hasBeenSubmitted()
-            && ((isset($form->valid) && $form->valid === true)
-                || $form->isValid())
-        ) {
-            $this->redirectNow($url);
-        }
-    }
 }
diff --git a/library/Reporting/Web/Forms/ReportForm.php b/library/Reporting/Web/Forms/ReportForm.php
@@ -17,16 +17,26 @@ class ReportForm extends CompatForm
     use Database;
     use ProvidedReports;
 
-    /** @var bool Hack to disable the {@link onSuccess()} code upon deletion of the report */
-    protected $callOnSuccess;
-
     protected $id;
 
-    public function setId($id)
+    /**
+     * Create a new form instance with the given report id
+     *
+     * @param $id
+     *
+     * @return static
+     */
+    public static function fromId($id): self
     {
-        $this->id = $id;
+        $form = new static();
+        $form->id = $id;
+
+        return $form;
+    }
 
-        return $this;
+    public function hasBeenSubmitted(): bool
+    {
+        return $this->hasBeenSent() && ($this->getPopulatedValue('submit') || $this->getPopulatedValue('remove'));
     }
 
     protected function assemble()
@@ -97,28 +107,19 @@ protected function assemble()
             ]);
             $this->registerElement($removeButton);
             $this->getElement('submit')->getWrapper()->prepend($removeButton);
-
-            if ($removeButton->hasBeenPressed()) {
-                $this->getDb()->delete('report', ['id = ?' => $this->id]);
-
-                // Stupid cheat because ipl/html is not capable of multiple submit buttons
-                $this->getSubmitButton()->setValue($this->getSubmitButton()->getButtonLabel());
-                $this->callOnSuccess = false;
-                $this->valid = true;
-
-                return;
-            }
         }
     }
 
     public function onSuccess()
     {
-        if ($this->callOnSuccess === false) {
+        $db = $this->getDb();
+
+        if ($this->getPopulatedValue('remove')) {
+            $db->delete('report', ['id = ?' => $this->id]);
+
             return;
         }
 
-        $db = $this->getDb();
-
         $values = $this->getValues();
 
         $now = time() * 1000;

diff --git a/library/Reporting/Web/Forms/ScheduleForm.php b/library/Reporting/Web/Forms/ScheduleForm.php
@@ -8,6 +8,7 @@
 use Icinga\Application\Version;
 use Icinga\Authentication\Auth;
 use Icinga\Module\Reporting\Database;
+use Icinga\Module\Reporting\Hook\ActionHook;
 use Icinga\Module\Reporting\ProvidedActions;
 use Icinga\Module\Reporting\Report;
 use Icinga\Module\Reporting\Web\Flatpickr;
@@ -25,36 +26,56 @@ class ScheduleForm extends CompatForm
     /** @var Report */
     protected $report;
 
+    /** @var int */
     protected $id;
 
-    public function setReport(Report $report)
+    /**
+     * Create a new form instance with the given report
+     *
+     * @param Report $report
+     *
+     * @return static
+     */
+    public static function fromReport(Report $report): self
     {
-        $this->report = $report;
+        $form = new static();
+
+        $form->report = $report;
 
         $schedule = $report->getSchedule();
 
         if ($schedule !== null) {
-            $this->setId($schedule->getId());
+            $form->setId($schedule->getId());
 
             $values = [
                     'start'     => $schedule->getStart()->format('Y-m-d\\TH:i:s'),
                     'frequency' => $schedule->getFrequency(),
                     'action'    => $schedule->getAction()
                 ] + $schedule->getConfig();
 
-            $this->populate($values);
+            $form->populate($values);
         }
 
-        return $this;
+        return $form;
     }
 
-    public function setId($id)
+    /**
+     * @param int $id
+     *
+     * @return $this
+     */
+    public function setId(int $id): ScheduleForm
     {
         $this->id = $id;
 
         return $this;
     }
 
+    public function hasBeenSubmitted(): bool
+    {
+        return $this->hasBeenSent() && ($this->getPopulatedValue('submit') || $this->getPopulatedValue('remove'));
+    }
+
     protected function assemble()
     {
         $this->setDefaultElementDecorator(new CompatDecorator());
@@ -100,7 +121,7 @@ protected function assemble()
             $config = new Form();
 //            $config->populate($this->getValues());
 
-            /** @var \Icinga\Module\Reporting\Hook\ActionHook $action */
+            /** @var ActionHook $action */
             $action = new $values['action']();
 
             $action->initConfigForm($config, $this->report);
@@ -123,23 +144,19 @@ protected function assemble()
             ]);
             $this->registerElement($removeButton);
             $this->getElement('submit')->getWrapper()->prepend($removeButton);
-
-            if ($removeButton->hasBeenPressed()) {
-                $this->getDb()->delete('schedule', ['id = ?' => $this->id]);
-
-                // Stupid cheat because ipl/html is not capable of multiple submit buttons
-                $this->getSubmitButton()->setValue($this->getSubmitButton()->getButtonLabel());
-                $this->valid = true;
-
-                return;
-            }
         }
     }
 
     public function onSuccess()
     {
         $db = $this->getDb();
 
+        if ($this->getPopulatedValue('remove')) {
+            $db->delete('schedule', ['id = ?' => $this->id]);
+
+            return;
+        }
+
         $values = $this->getValues();
 
         $now = time() * 1000;