ReportForm: Check whether the report name is unique

The form now requires a database instance (RetryConnection), which should be specified as a constructor parameter in order to keep the form dependency-free.
Icinga · Sep 26, 2024 · 6e1fa2c · 6e1fa2c
1 parent 239dd71
commit 6e1fa2c
Show file tree

Hide file tree

Showing 3 changed files with 37 additions and 7 deletions.
diff --git a/application/controllers/ReportController.php b/application/controllers/ReportController.php
@@ -112,7 +112,7 @@ public function cloneAction(): void
             $values[$name] = $value;
         }
 
-        $form = (new ReportForm())
+        $form = (new ReportForm(Database::get()))
             ->setSubmitButtonLabel($this->translate('Clone Report'))
             ->setAction((string) Url::fromRequest())
             ->populate($values)
@@ -148,7 +148,7 @@ public function editAction(): void
             $values[$name] = $value;
         }
 
-        $form = ReportForm::fromId($this->report->getId())
+        $form = ReportForm::fromId($this->report->getId(), Database::get())
             ->setAction((string) Url::fromRequest())
             ->populate($values)
             ->on(ReportForm::ON_SUCCESS, function (ReportForm $form) {

diff --git a/application/controllers/ReportsController.php b/application/controllers/ReportsController.php
@@ -112,7 +112,7 @@ public function newAction(): void
                 break;
         }
 
-        $form = (new ReportForm())
+        $form = (new ReportForm(Database::get()))
             ->setAction((string) Url::fromRequest())
             ->setRenderCreateAndShowButton($class !== null)
             ->populate([

diff --git a/library/Reporting/Web/Forms/ReportForm.php b/library/Reporting/Web/Forms/ReportForm.php
@@ -6,9 +6,12 @@
 
 use Icinga\Authentication\Auth;
 use Icinga\Module\Reporting\Database;
+use Icinga\Module\Reporting\Model\Report;
 use Icinga\Module\Reporting\ProvidedReports;
+use Icinga\Module\Reporting\RetryConnection;
 use ipl\Html\Form;
 use ipl\Html\HtmlDocument;
+use ipl\Stdlib\Filter;
 use ipl\Validator\CallbackValidator;
 use ipl\Web\Compat\CompatForm;
 
@@ -24,16 +27,25 @@ class ReportForm extends CompatForm
     /** @var bool Whether to render the create and show submit button (is only used from DB Web's object detail) */
     protected $renderCreateAndShowButton = false;
 
+    /** @var RetryConnection */
+    protected $db;
+
+    public function __construct(RetryConnection $db)
+    {
+        $this->db = $db;
+    }
+
     /**
      * Create a new form instance with the given report id
      *
-     * @param $id
+     * @param int $id
+     * @param RetryConnection $db
      *
      * @return static
      */
-    public static function fromId($id): self
+    public static function fromId(int $id, RetryConnection $db): self
     {
-        $form = new static();
+        $form = new static($db);
         $form->id = $id;
 
         return $form;
@@ -106,14 +118,32 @@ protected function assemble()
             ),
             'validators' => [
                 'Callback' => function ($value, CallbackValidator $validator) {
-                    if ($value !== null && strpos($value, '..') !== false) {
+                    if (strpos($value, '..') !== false) {
                         $validator->addMessage(
                             $this->translate('Double dots are not allowed in the report name')
                         );
 
                         return false;
                     }
 
+                    $filter = Filter::all(Filter::equal('name', $value));
+                    if ($this->id) {
+                        $filter->add(Filter::unequal('id', $this->id));
+                    }
+
+                    $report = Report::on($this->db)
+                        ->columns('1')
+                        ->filter($filter)
+                        ->first();
+
+                    if ($report !== null) {
+                        $validator->addMessage(
+                            $this->translate('A report with this name already exists')
+                        );
+
+                        return false;
+                    }
+
                     return true;
                 }
             ]