From a9c0075dc0787d4f68f2d7882f9e756ab281784d Mon Sep 17 00:00:00 2001 From: Johan Cwiklinski Date: Thu, 16 May 2024 07:55:42 +0200 Subject: [PATCH] Strict types declaration, several fixes --- .composer-require-checker.config.json | 1 + _config.inc.php | 2 ++ _define.php | 2 ++ _routes.php | 2 ++ lib/GaletteAuto/AbstractObject.php | 12 ++++--- lib/GaletteAuto/Auto.php | 35 +++++++++++-------- lib/GaletteAuto/Autos.php | 2 ++ lib/GaletteAuto/Body.php | 2 ++ lib/GaletteAuto/Brand.php | 2 ++ lib/GaletteAuto/Color.php | 2 ++ lib/GaletteAuto/Controllers/Controller.php | 18 ++++++---- .../Controllers/Crud/ModelsController.php | 6 ++-- .../Controllers/Crud/PropertiesController.php | 4 ++- lib/GaletteAuto/Filters/AutosList.php | 2 ++ lib/GaletteAuto/Filters/ModelsList.php | 2 ++ lib/GaletteAuto/Filters/PropertiesList.php | 2 ++ lib/GaletteAuto/Finition.php | 2 ++ lib/GaletteAuto/History.php | 2 ++ lib/GaletteAuto/Model.php | 4 ++- lib/GaletteAuto/Picture.php | 4 ++- lib/GaletteAuto/PluginGaletteAuto.php | 2 ++ lib/GaletteAuto/Repository/Models.php | 2 ++ lib/GaletteAuto/State.php | 2 ++ lib/GaletteAuto/Transmission.php | 2 ++ templates/default/history.html.twig | 2 +- templates/default/vehicles.html.twig | 29 +++++++-------- tests/GaletteAuto/tests/units/Auto.php | 6 ++-- 27 files changed, 101 insertions(+), 52 deletions(-) diff --git a/.composer-require-checker.config.json b/.composer-require-checker.config.json index 7163036..c4190d9 100644 --- a/.composer-require-checker.config.json +++ b/.composer-require-checker.config.json @@ -30,6 +30,7 @@ "GALETTE_CARD_ROWS", "GALETTE_CARD_WIDTH", "GALETTE_COMPAT_VERSION", + "GALETTE_SYSCONFIG_PATH", "GALETTE_CONFIG_PATH", "GALETTE_DATA_PATH", "GALETTE_DB_VERSION", diff --git a/_config.inc.php b/_config.inc.php index 2586ced..b0c4de5 100644 --- a/_config.inc.php +++ b/_config.inc.php @@ -19,4 +19,6 @@ * along with Galette. If not, see . */ +declare(strict_types=1); + define('AUTO_PREFIX', 'auto_'); diff --git a/_define.php b/_define.php index ddec28a..bd89cbc 100644 --- a/_define.php +++ b/_define.php @@ -19,6 +19,8 @@ * along with Galette. If not, see . */ +declare(strict_types=1); + $this->register( 'Galette Auto', //Name 'Plugin to manage Automobile clubs', //Short description diff --git a/_routes.php b/_routes.php index dc55304..d228365 100644 --- a/_routes.php +++ b/_routes.php @@ -19,6 +19,8 @@ * along with Galette. If not, see . */ +declare(strict_types=1); + use GaletteAuto\Controllers\Controller; use GaletteAuto\Controllers\Crud\PropertiesController; use GaletteAuto\Controllers\Crud\ModelsController; diff --git a/lib/GaletteAuto/AbstractObject.php b/lib/GaletteAuto/AbstractObject.php index 4f9fea9..86718f8 100644 --- a/lib/GaletteAuto/AbstractObject.php +++ b/lib/GaletteAuto/AbstractObject.php @@ -19,6 +19,8 @@ * along with Galette. If not, see . */ +declare(strict_types=1); + namespace GaletteAuto; use Analog\Analog; @@ -45,7 +47,7 @@ abstract class AbstractObject protected Db $zdb; protected ?int $id; - protected string $value; + protected ?string $value; protected ?PropertiesList $filters = null; private int $count; @@ -168,7 +170,7 @@ public function store(bool $new = false): bool } catch (\Exception $e) { Analog::log( '[' . get_class($this) . '] Cannot store ' . $this->name . - ' values `' . $this->id . '`, `' . $this->value . '` | ' . + ' values `' . ($this->id ?? '') . '`, `' . $this->value . '` | ' . $e->getMessage(), Analog::WARNING ); @@ -236,14 +238,14 @@ abstract public function getRouteName(): string; */ public function __get(string $name): mixed { - if (isset($this->$name)) { - return $this->$name; + if (property_exists($this, $name)) { + return $this->$name ?? null; } else { Analog::log( '[' . get_class($this) . '] Unable to retrieve `' . $name . '`', Analog::INFO ); - return false; + throw new \RuntimeException('Unable to retrieve `' . $name . '`'); } } diff --git a/lib/GaletteAuto/Auto.php b/lib/GaletteAuto/Auto.php index c032ce9..053b848 100644 --- a/lib/GaletteAuto/Auto.php +++ b/lib/GaletteAuto/Auto.php @@ -19,6 +19,8 @@ * along with Galette. If not, see . */ +declare(strict_types=1); + namespace GaletteAuto; use ArrayObject; @@ -53,7 +55,8 @@ * @property Transmission $transmission * @property Finition $finition * @property Model $model - * @property Adherent|int $owner + * @property int $owner_id + * @property Adherent $owner * @property Picture $picture * @property History $history */ @@ -125,6 +128,7 @@ class Auto private Body $body; private History $history; private State $state; + private int $owner_id; private Adherent $owner; public const FUEL_PETROL = 1; @@ -271,7 +275,8 @@ private function loadFromRS(ArrayObject $r): void $bpk = Body::PK; $this->body->load((int)$r->$bpk); $opk = Adherent::PK; - $this->owner->load((int)$r->$opk); + $this->owner_id = (int)$r->$opk; + $this->owner->load($this->owner_id); $spk = State::PK; $this->state->load((int)$r->$spk); $this->history->load((int)$this->id); @@ -345,7 +350,7 @@ public function store(bool $new = false): bool switch ($v) { case 'string': case 'date': - $values[$k] = $this->$propName; + $values[$k] = $this->$propName ?? null; break; case 'integer': $values[$k] = ( @@ -501,7 +506,8 @@ public function hasPicture(): bool */ public function appropriateCar(Login $login): void { - $this->owner->load($login->id); + $this->owner_id = $login->id; + $this->owner->load($this->owner_id); } /** @@ -545,7 +551,7 @@ public function __get(string $name): mixed case 'first_registration_date': case 'first_circulation_date': case 'creation_date': - if ($this->$name != '') { + if (isset($this->$name)) { try { $d = new \DateTime($this->$name); return $d->format(_T("Y-m-d")); @@ -559,12 +565,11 @@ public function __get(string $name): mixed return $this->$name; } } - break; + return null; case 'picture': return $this->picture; default: - return $this->$name; - break; + return $this->$name ?? ''; } } @@ -604,8 +609,9 @@ public function __set(string $name, mixed $value): void case 'body': $this->body->load((int)$value); break; - case 'owner': - $this->owner->load((int)$value); + case 'owner_id': + $this->owner_id = (int)$value; + $this->owner->load($this->owner_id); break; case 'state': $this->state->load((int)$value); @@ -739,7 +745,7 @@ public function check(array $post): bool //constants case 'fuel': if (in_array($value, array_keys($this->listFuels()))) { - $this->fuel = $value; + $this->fuel = (int)$value; } else { $this->errors[] = _T("- You must choose a fuel in the list", "auto"); } @@ -752,7 +758,7 @@ public function check(array $post): bool case 'body': case 'state': if ($value > 0) { - $this->$prop->load($value); + $this->$prop->load((int)$value); } else { $class = 'GaletteAuto\\' . ucwords($prop); $name = $class::FIELD; @@ -763,11 +769,12 @@ public function check(array $post): bool ); } break; - case 'owner': + case 'owner_id': if (isset($post['change_owner']) || !isset($this->id)) { $value = (int)$value; if ($value > 0) { - $this->$prop->load($value); + $this->owner_id = $value; + $this->owner->load($value); } else { $this->errors[] = _T("- you must attach an owner to this car", "auto"); } diff --git a/lib/GaletteAuto/Autos.php b/lib/GaletteAuto/Autos.php index 510a974..b500d6f 100644 --- a/lib/GaletteAuto/Autos.php +++ b/lib/GaletteAuto/Autos.php @@ -19,6 +19,8 @@ * along with Galette. If not, see . */ +declare(strict_types=1); + namespace GaletteAuto; use Analog\Analog; diff --git a/lib/GaletteAuto/Body.php b/lib/GaletteAuto/Body.php index 02e9890..bcc1fda 100644 --- a/lib/GaletteAuto/Body.php +++ b/lib/GaletteAuto/Body.php @@ -19,6 +19,8 @@ * along with Galette. If not, see . */ +declare(strict_types=1); + namespace GaletteAuto; use Galette\Core\Db; diff --git a/lib/GaletteAuto/Brand.php b/lib/GaletteAuto/Brand.php index a880475..60beae9 100644 --- a/lib/GaletteAuto/Brand.php +++ b/lib/GaletteAuto/Brand.php @@ -19,6 +19,8 @@ * along with Galette. If not, see . */ +declare(strict_types=1); + namespace GaletteAuto; use Galette\Core\Db; diff --git a/lib/GaletteAuto/Color.php b/lib/GaletteAuto/Color.php index 8daa1ef..883b5e5 100644 --- a/lib/GaletteAuto/Color.php +++ b/lib/GaletteAuto/Color.php @@ -19,6 +19,8 @@ * along with Galette. If not, see . */ +declare(strict_types=1); + namespace GaletteAuto; use Galette\Core\Db; diff --git a/lib/GaletteAuto/Controllers/Controller.php b/lib/GaletteAuto/Controllers/Controller.php index 02fdbcf..818efc4 100644 --- a/lib/GaletteAuto/Controllers/Controller.php +++ b/lib/GaletteAuto/Controllers/Controller.php @@ -19,6 +19,8 @@ * along with Galette. If not, see . */ +declare(strict_types=1); + namespace GaletteAuto\Controllers; use ArrayObject; @@ -299,7 +301,7 @@ public function showAddEditVehicle(Request $request, Response $response, string isset($get['id_adh']) && ($this->login->isAdmin() || $this->login->isStaff()) ) { - $auto->owner = (int)$get['id_adh']; + $auto->owner_id = (int)$get['id_adh']; } else { $auto->appropriateCar($this->login); } @@ -328,7 +330,7 @@ public function showAddEditVehicle(Request $request, Response $response, string 'require_calendar' => true, 'require_dialog' => true, 'car' => $auto, - 'models' => $models->getList($auto->model->brand->id), + 'models' => $models->getList($auto->model->brand->id ?? null), 'js_init_models' => !isset($auto->model->brand), 'brands' => $auto->model->brand->getList(), 'colors' => $auto->color->getList(), @@ -361,6 +363,7 @@ public function showAddEditVehicle(Request $request, Response $response, string if (count($members)) { $params['members']['list'] = $members; } + $params['autocomplete'] = true; // display page $this->view->render( @@ -412,7 +415,7 @@ public function doAddEditVehicle(Request $request, Response $response, string $a { $post = $request->getParsedBody(); - $is_new = ($action === 'add'); + $is_new = ($action === 'add' || $action === 'new'); // initialize warnings $error_detected = array(); @@ -424,7 +427,7 @@ public function doAddEditVehicle(Request $request, Response $response, string $a $auto = new Auto($this->plugins, $this->zdb); if (!$is_new) { - $auto->load($post[Auto::PK]); + $auto->load((int)$post[Auto::PK]); } $res = $auto->check($post); @@ -488,14 +491,15 @@ public function doAddEditVehicle(Request $request, Response $response, string $a */ public function vehicleHistory(Request $request, Response $response, int $id): Response { + $apk = Auto::PK; $history = new History($this->zdb, $id); - $auto = new Auto($this->plugins, $this->zdb, $history->{Auto::PK}); + $auto = new Auto($this->plugins, $this->zdb); + $auto->load($history->$apk); $this->checkAclsFor($response, $auto->owner->id); - $apk = Auto::PK; $params = [ 'entries' => $history->getEntries(), - 'page_title' => str_replace('%d', $history->$apk, _T("History of car #%d", "auto")), + 'page_title' => str_replace('%d', (string)$history->$apk, _T("History of car #%d", "auto")), 'mode' => $request->getHeaderLine('X-Requested-With') === 'XMLHttpRequest' ? 'ajax' : '' ]; diff --git a/lib/GaletteAuto/Controllers/Crud/ModelsController.php b/lib/GaletteAuto/Controllers/Crud/ModelsController.php index f0cd43d..67cb835 100644 --- a/lib/GaletteAuto/Controllers/Crud/ModelsController.php +++ b/lib/GaletteAuto/Controllers/Crud/ModelsController.php @@ -19,6 +19,8 @@ * along with Galette. If not, see . */ +declare(strict_types=1); + namespace GaletteAuto\Controllers\Crud; use DI\Attribute\Inject; @@ -185,9 +187,9 @@ public function edit(Request $request, Response $response, int $id = null, strin { $model = new Model($this->zdb); - if ($this->session->automodel !== null) { + if ($this->session->auto_model !== null) { $model->check($this->session->auto_model); - $this->session->automodel = null; + unset($this->session->auto_model); } $model_id = null; diff --git a/lib/GaletteAuto/Controllers/Crud/PropertiesController.php b/lib/GaletteAuto/Controllers/Crud/PropertiesController.php index b2a385f..c9f1b98 100644 --- a/lib/GaletteAuto/Controllers/Crud/PropertiesController.php +++ b/lib/GaletteAuto/Controllers/Crud/PropertiesController.php @@ -19,6 +19,8 @@ * along with Galette. If not, see . */ +declare(strict_types=1); + namespace GaletteAuto\Controllers\Crud; use DI\Attribute\Inject; @@ -406,7 +408,7 @@ public function doPropertyEdit( if (!$is_new) { if (isset($post[$object->pk])) { - $object->load($post[$object->pk]); + $object->load((int)$post[$object->pk]); } else { $error_detected[] = _T("- No id provided for modifying this record! (internal)", "auto"); diff --git a/lib/GaletteAuto/Filters/AutosList.php b/lib/GaletteAuto/Filters/AutosList.php index 1a5ab1c..390c9c8 100644 --- a/lib/GaletteAuto/Filters/AutosList.php +++ b/lib/GaletteAuto/Filters/AutosList.php @@ -19,6 +19,8 @@ * along with Galette. If not, see . */ +declare(strict_types=1); + namespace GaletteAuto\Filters; use Galette\Core\Pagination; diff --git a/lib/GaletteAuto/Filters/ModelsList.php b/lib/GaletteAuto/Filters/ModelsList.php index f56e05f..9c007d3 100644 --- a/lib/GaletteAuto/Filters/ModelsList.php +++ b/lib/GaletteAuto/Filters/ModelsList.php @@ -19,6 +19,8 @@ * along with Galette. If not, see . */ +declare(strict_types=1); + namespace GaletteAuto\Filters; use Galette\Core\Pagination; diff --git a/lib/GaletteAuto/Filters/PropertiesList.php b/lib/GaletteAuto/Filters/PropertiesList.php index 0be316b..2ff6c8b 100644 --- a/lib/GaletteAuto/Filters/PropertiesList.php +++ b/lib/GaletteAuto/Filters/PropertiesList.php @@ -19,6 +19,8 @@ * along with Galette. If not, see . */ +declare(strict_types=1); + namespace GaletteAuto\Filters; use Galette\Core\Pagination; diff --git a/lib/GaletteAuto/Finition.php b/lib/GaletteAuto/Finition.php index a51989a..36d575e 100644 --- a/lib/GaletteAuto/Finition.php +++ b/lib/GaletteAuto/Finition.php @@ -19,6 +19,8 @@ * along with Galette. If not, see . */ +declare(strict_types=1); + namespace GaletteAuto; use Galette\Core\Db; diff --git a/lib/GaletteAuto/History.php b/lib/GaletteAuto/History.php index dbfb6bb..bd5eb58 100644 --- a/lib/GaletteAuto/History.php +++ b/lib/GaletteAuto/History.php @@ -19,6 +19,8 @@ * along with Galette. If not, see . */ +declare(strict_types=1); + namespace GaletteAuto; use Analog\Analog; diff --git a/lib/GaletteAuto/Model.php b/lib/GaletteAuto/Model.php index b287820..2b58ad1 100644 --- a/lib/GaletteAuto/Model.php +++ b/lib/GaletteAuto/Model.php @@ -19,6 +19,8 @@ * along with Galette. If not, see . */ +declare(strict_types=1); + namespace GaletteAuto; use Analog\Analog; @@ -194,7 +196,7 @@ public function delete(array $ids): bool */ public function __get(string $name): mixed { - return $this->$name; + return $this->$name ?? null; } /** diff --git a/lib/GaletteAuto/Picture.php b/lib/GaletteAuto/Picture.php index 5f85174..6323cc1 100644 --- a/lib/GaletteAuto/Picture.php +++ b/lib/GaletteAuto/Picture.php @@ -19,6 +19,8 @@ * along with Galette. If not, see . */ +declare(strict_types=1); + namespace GaletteAuto; use Analog\Analog; @@ -78,7 +80,7 @@ public function __construct(Plugins $plugins, mixed $id_adh = null) */ protected function getDefaultPicture(): void { - $this->file_path = realpath( + $this->file_path = (string)realpath( $this->plugins->getTemplatesPathFromName('Galette Auto') . '/../../webroot/images/1f698.png' ); diff --git a/lib/GaletteAuto/PluginGaletteAuto.php b/lib/GaletteAuto/PluginGaletteAuto.php index 931b3fc..3fa9ecf 100644 --- a/lib/GaletteAuto/PluginGaletteAuto.php +++ b/lib/GaletteAuto/PluginGaletteAuto.php @@ -19,6 +19,8 @@ * along with Galette. If not, see . */ +declare(strict_types=1); + namespace GaletteAuto; use Galette\Core\Login; diff --git a/lib/GaletteAuto/Repository/Models.php b/lib/GaletteAuto/Repository/Models.php index 2052a28..8e9c47c 100644 --- a/lib/GaletteAuto/Repository/Models.php +++ b/lib/GaletteAuto/Repository/Models.php @@ -19,6 +19,8 @@ * along with Galette. If not, see . */ +declare(strict_types=1); + namespace GaletteAuto\Repository; use Galette\Core\Db; diff --git a/lib/GaletteAuto/State.php b/lib/GaletteAuto/State.php index cdf13a8..b12b248 100644 --- a/lib/GaletteAuto/State.php +++ b/lib/GaletteAuto/State.php @@ -19,6 +19,8 @@ * along with Galette. If not, see . */ +declare(strict_types=1); + namespace GaletteAuto; use Galette\Core\Db; diff --git a/lib/GaletteAuto/Transmission.php b/lib/GaletteAuto/Transmission.php index 3d5f802..87dcc20 100644 --- a/lib/GaletteAuto/Transmission.php +++ b/lib/GaletteAuto/Transmission.php @@ -19,6 +19,8 @@ * along with Galette. If not, see . */ +declare(strict_types=1); + namespace GaletteAuto; use Galette\Core\Db; diff --git a/templates/default/history.html.twig b/templates/default/history.html.twig index 086c833..5355cec 100644 --- a/templates/default/history.html.twig +++ b/templates/default/history.html.twig @@ -22,7 +22,7 @@ {% block content %} {% if entries|length > 0 %} - +
diff --git a/templates/default/vehicles.html.twig b/templates/default/vehicles.html.twig index c13fe49..1e86a9b 100644 --- a/templates/default/vehicles.html.twig +++ b/templates/default/vehicles.html.twig @@ -149,25 +149,20 @@

{% else %} - {% endif %} {% if login.isAdmin() or login.isStaff() %} - -
- - - - -
{{ _T("Search for name or ID and pick member") }}
-
- {% for k, v in members.list %} -
{{ v }}
- {% endfor %} -
-
- + {% set cclass = 'field' %} + {% if car.id %}{% set cclass = cclass ~ ' displaynone' %}{% endif %} + {% include 'components/forms/member_dropdown.html.twig' with { + 'required': true, + 'component_id': 'owner_id_elt', + 'id': 'owner_id', + 'label': _T("Owner", "auto"), + 'value': car.owner.id, + 'component_class': cclass + } %} {% else %} @@ -347,10 +342,10 @@ {# Popup for owner change #} $('#change_owner-checkbox').checkbox({ onChecked: function() { - $('#owner_elt').removeClass('displaynone'); + $('#owner_id_elt').removeClass('displaynone'); }, onUnchecked: function() { - $('#owner_elt').addClass('displaynone'); + $('#owner_id_elt').addClass('displaynone'); } }); diff --git a/tests/GaletteAuto/tests/units/Auto.php b/tests/GaletteAuto/tests/units/Auto.php index b9a4570..b3b980b 100644 --- a/tests/GaletteAuto/tests/units/Auto.php +++ b/tests/GaletteAuto/tests/units/Auto.php @@ -177,7 +177,7 @@ public function testCrud(): void 'finition' => $finition_id, 'state' => $state_id, 'transmission' => $transmission_id, - 'owner' => $adh->id, + 'owner_id' => $adh->id, ]; $check = $auto->check($data); $this->assertSame([], $auto->getErrors()); @@ -238,7 +238,7 @@ public function testCrud(): void 'finition' => $finition_id, 'state' => $state_id, 'transmission' => $transmission_id, - 'owner' => $adh2->id, + 'owner_id' => $adh2->id, 'change_owner' => true, ]; $check = $auto->check($data); @@ -275,7 +275,7 @@ public function testCrud(): void 'finition' => $finition_id, 'state' => $state_id, 'transmission' => $transmission_id, - 'owner' => $adh->id, + 'owner_id' => $adh->id, ]; $check = $auto->check($data); $this->assertSame([], $auto->getErrors());