From 6e1fa2c8f3be3f206fea124ac1f681b3f38c1f1e Mon Sep 17 00:00:00 2001 From: Sukhwinder Dhillon Date: Mon, 22 Jul 2024 12:23:33 +0200 Subject: [PATCH 1/2] 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. --- application/controllers/ReportController.php | 4 +- application/controllers/ReportsController.php | 2 +- library/Reporting/Web/Forms/ReportForm.php | 38 +++++++++++++++++-- 3 files changed, 37 insertions(+), 7 deletions(-) diff --git a/application/controllers/ReportController.php b/application/controllers/ReportController.php index cc9d906..2f01851 100644 --- 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 index c3c3228..d712271 100644 --- 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 index 83aa179..c378082 100644 --- 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,7 +118,7 @@ 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') ); @@ -114,6 +126,24 @@ protected function assemble() 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; } ] From 137f4ac82d8792dfc3cc67e39e153fb6373821ba Mon Sep 17 00:00:00 2001 From: Sukhwinder Dhillon Date: Mon, 22 Jul 2024 14:17:36 +0200 Subject: [PATCH 2/2] Add phpDoc and update phpstan-baseline --- library/Reporting/Web/Forms/ReportForm.php | 6 +++--- phpstan-baseline.neon | 25 +++++----------------- 2 files changed, 8 insertions(+), 23 deletions(-) diff --git a/library/Reporting/Web/Forms/ReportForm.php b/library/Reporting/Web/Forms/ReportForm.php index c378082..709bce3 100644 --- a/library/Reporting/Web/Forms/ReportForm.php +++ b/library/Reporting/Web/Forms/ReportForm.php @@ -19,6 +19,7 @@ class ReportForm extends CompatForm { use ProvidedReports; + /** @var ?int */ protected $id; /** @var string Label to use for the submit button */ @@ -107,7 +108,7 @@ public function hasBeenSubmitted(): bool ); } - protected function assemble() + protected function assemble(): void { $this->addElement('text', 'name', [ 'required' => true, @@ -179,7 +180,6 @@ protected function assemble() if (isset($values['reportlet'])) { $config = new Form(); -// $config->populate($this->getValues()); /** @var \Icinga\Module\Reporting\Hook\ReportHook $reportlet */ $reportlet = new $values['reportlet'](); @@ -218,7 +218,7 @@ protected function assemble() } } - public function onSuccess() + public function onSuccess(): void { $db = Database::get(); diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 36b8734..581e1cc 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -145,6 +145,11 @@ parameters: count: 1 path: application/forms/SelectBackendForm.php + - + message: "#^Call to an undefined method React\\\\Promise\\\\PromiseInterface\\:\\:otherwise\\(\\)\\.$#" + count: 1 + path: library/Reporting/Actions/SendMail.php + - message: "#^Method Icinga\\\\Module\\\\Reporting\\\\Actions\\\\SendMail\\:\\:execute\\(\\) has no return type specified\\.$#" count: 1 @@ -680,31 +685,11 @@ parameters: count: 1 path: library/Reporting/Web/Forms/ReportForm.php - - - message: "#^Method Icinga\\\\Module\\\\Reporting\\\\Web\\\\Forms\\\\ReportForm\\:\\:assemble\\(\\) has no return type specified\\.$#" - count: 1 - path: library/Reporting/Web/Forms/ReportForm.php - - - - message: "#^Method Icinga\\\\Module\\\\Reporting\\\\Web\\\\Forms\\\\ReportForm\\:\\:fromId\\(\\) has parameter \\$id with no type specified\\.$#" - count: 1 - path: library/Reporting/Web/Forms/ReportForm.php - - message: "#^Method Icinga\\\\Module\\\\Reporting\\\\Web\\\\Forms\\\\ReportForm\\:\\:listReports\\(\\) has no return type specified\\.$#" count: 1 path: library/Reporting/Web/Forms/ReportForm.php - - - message: "#^Method Icinga\\\\Module\\\\Reporting\\\\Web\\\\Forms\\\\ReportForm\\:\\:onSuccess\\(\\) has no return type specified\\.$#" - count: 1 - path: library/Reporting/Web/Forms/ReportForm.php - - - - message: "#^Property Icinga\\\\Module\\\\Reporting\\\\Web\\\\Forms\\\\ReportForm\\:\\:\\$id has no type specified\\.$#" - count: 1 - path: library/Reporting/Web/Forms/ReportForm.php - - message: "#^Call to an undefined method object\\:\\:execute\\(\\)\\.$#" count: 1