From eed98019ce96398ee1319d6550d576fea7b0558a Mon Sep 17 00:00:00 2001 From: Johan Cwiklinski Date: Sat, 26 Sep 2026 10:50:25 +0200 Subject: [PATCH 1/6] Give vehicle properties explicit accessors instead of magic ones --- lib/GaletteAuto/AbstractObject.php | 90 ++++++++++--------- lib/GaletteAuto/Auto.php | 14 +-- lib/GaletteAuto/Body.php | 15 ---- lib/GaletteAuto/Brand.php | 20 ----- lib/GaletteAuto/Color.php | 15 ---- lib/GaletteAuto/Controllers/Controller.php | 2 +- .../Controllers/Crud/PropertiesController.php | 14 +-- lib/GaletteAuto/Finition.php | 15 ---- lib/GaletteAuto/History.php | 4 +- lib/GaletteAuto/Model.php | 2 +- lib/GaletteAuto/State.php | 15 ---- lib/GaletteAuto/Transmission.php | 15 ---- templates/default/model.html.twig | 2 +- templates/default/models_list.html.twig | 6 +- templates/default/object.html.twig | 11 ++- templates/default/object_list.html.twig | 5 +- templates/default/object_show.html.twig | 9 +- .../default/public_vehicles_list.html.twig | 6 +- templates/default/vehicles.html.twig | 12 +-- templates/default/vehicles_list.html.twig | 2 +- .../Controllers/tests/units/Controller.php | 10 +-- .../tests/units/ModelsController.php | 8 +- .../tests/units/PropertiesController.php | 22 ++--- tests/GaletteAuto/tests/units/Auto.php | 28 +++--- tests/GaletteAuto/tests/units/Body.php | 10 +-- tests/GaletteAuto/tests/units/Brand.php | 10 +-- tests/GaletteAuto/tests/units/Color.php | 10 +-- tests/GaletteAuto/tests/units/Finition.php | 10 +-- tests/GaletteAuto/tests/units/Model.php | 8 +- tests/GaletteAuto/tests/units/State.php | 10 +-- .../GaletteAuto/tests/units/Transmission.php | 10 +-- 31 files changed, 159 insertions(+), 251 deletions(-) diff --git a/lib/GaletteAuto/AbstractObject.php b/lib/GaletteAuto/AbstractObject.php index 20c8242..e051035 100644 --- a/lib/GaletteAuto/AbstractObject.php +++ b/lib/GaletteAuto/AbstractObject.php @@ -10,6 +10,7 @@ namespace GaletteAuto; +use ArrayObject; use Analog\Analog; use Laminas\Db\Sql\Expression; use Laminas\Db\Sql\Select; @@ -21,9 +22,6 @@ * Automobile Object abstract class for galette Auto plugin * * @author Johan Cwiklinski - * - * @property int $id - * @property string $value */ abstract class AbstractObject { @@ -33,8 +31,8 @@ abstract class AbstractObject private string $name; protected Db $zdb; - protected ?int $id; - protected ?string $value; + protected ?int $id = null; + protected ?string $value = null; protected ?PropertiesList $filters = null; private int $count; @@ -64,7 +62,7 @@ public function __construct(Db $zdb, string $table, string $pk, string $field, s /** * Get the list * - * @return array> + * @return array> */ public function getList(): array { @@ -101,12 +99,11 @@ public function load(int $id): bool ] ); - $results = $this->zdb->execute($select); - $result = $results->current(); - $pk = $this->pk; - $this->id = (int)$result->$pk; - $field = $this->field; - $this->value = $result->$field; + $result = $this->zdb->execute($select)->current(); + if (!$result instanceof ArrayObject) { + throw new \RuntimeException('Record not found'); + } + $this->loadFromRow($result); return true; } catch (\Exception $e) { @@ -119,6 +116,18 @@ public function load(int $id): bool } } + /** + * Populate from a resultset row, which may come from a join + * + * @param ArrayObject $row Resultset row + */ + public function loadFromRow(ArrayObject $row): self + { + $this->id = (int)$row[$this->pk]; + $this->value = (string)$row[$this->field]; + return $this; + } + /** * Store current record * @@ -205,49 +214,46 @@ abstract public function getFieldLabel(): string; abstract public function getRouteName(): string; /** - * Global getter method - * - * @param string $name name of the property we want to retrieve - * - * @return mixed the called property + * Get record ID */ - public function __get(string $name): mixed + public function getId(): ?int { - if (property_exists($this, $name)) { - return $this->$name ?? null; - } else { - Analog::log( - '[' . get_class($this) . '] Unable to retrieve `' . $name . '`', - Analog::INFO - ); - throw new \RuntimeException('Unable to retrieve `' . $name . '`'); - } + return $this->id; } /** - * Global isset method - * Required for twig to access properties via __get - * - * @param string $name name of the property we want to retrieve + * Get record value */ - public function __isset(string $name): bool + public function getValue(): ?string { - return property_exists($this, $name); + return $this->value; } /** - * Global setter method + * Set record value * - * @param string $name name of the property we want to assign a value to - * @param mixed $value a relevant value for the property + * @param string $value Value */ - public function __set(string $name, mixed $value): void + public function setValue(string $value): self { - switch ($name) { - case 'value': - $this->value = $value; - break; - } + $this->value = $value; + return $this; + } + + /** + * Get primary key field name + */ + public function getPk(): string + { + return $this->pk; + } + + /** + * Get value field name + */ + public function getField(): string + { + return $this->field; } /** diff --git a/lib/GaletteAuto/Auto.php b/lib/GaletteAuto/Auto.php index 39d257b..6c5cbc8 100644 --- a/lib/GaletteAuto/Auto.php +++ b/lib/GaletteAuto/Auto.php @@ -304,19 +304,19 @@ public function store(bool $new = false): bool case self::PK: break; case Color::PK: - $values[$k] = $this->color->id; + $values[$k] = $this->color->getId(); break; case Body::PK: - $values[$k] = $this->body->id; + $values[$k] = $this->body->getId(); break; case State::PK: - $values[$k] = $this->state->id; + $values[$k] = $this->state->getId(); break; case Transmission::PK: - $values[$k] = $this->transmission->id; + $values[$k] = $this->transmission->getId(); break; case Finition::PK: - $values[$k] = $this->finition->id; + $values[$k] = $this->finition->getId(); break; case Model::PK: $values[$k] = $this->model->id; @@ -541,9 +541,9 @@ public function __get(string $name): mixed case Adherent::PK: return $this->owner->id; case Color::PK: - return $this->color->id; + return $this->color->getId(); case State::PK: - return $this->state->id; + return $this->state->getId(); case 'car_registration': return $this->registration; case 'first_registration_date': diff --git a/lib/GaletteAuto/Body.php b/lib/GaletteAuto/Body.php index d943504..92fbfab 100644 --- a/lib/GaletteAuto/Body.php +++ b/lib/GaletteAuto/Body.php @@ -58,21 +58,6 @@ public function getRouteName(): string return 'body'; } - /** - * Global getter method - * - * @param string $name name of the property we want to retrieve - * - * @return mixed the called property - */ - public function __get(string $name): mixed - { - if ($name == self::FIELD) { - return parent::__get('value'); - } else { - return parent::__get($name); - } - } /** * Get localized count string for object list diff --git a/lib/GaletteAuto/Brand.php b/lib/GaletteAuto/Brand.php index ea5caff..8290ff9 100644 --- a/lib/GaletteAuto/Brand.php +++ b/lib/GaletteAuto/Brand.php @@ -22,8 +22,6 @@ * @license http://www.gnu.org/licenses/gpl-3.0.html GPL License 3.0 or (at your option) any later version * @link https://galette.eu * @since Available since 0.7dev - 2009-03-16 - * - * @property int $id */ class Brand extends AbstractObject { @@ -66,24 +64,6 @@ public function getRouteName(): string return 'brand'; } - /** - * Global getter method - * - * @param string $name name of the property we want to retrieve - * - * @return mixed the called property - */ - public function __get(string $name): mixed - { - if ($name == self::FIELD) { - return parent::__get('value'); - } - if ($name == self::PK) { - return parent::__get('id'); - } else { - return parent::__get($name); - } - } /** * Get localized count string for object list diff --git a/lib/GaletteAuto/Color.php b/lib/GaletteAuto/Color.php index fe68489..8e826a6 100644 --- a/lib/GaletteAuto/Color.php +++ b/lib/GaletteAuto/Color.php @@ -58,21 +58,6 @@ public function getRouteName(): string return 'color'; } - /** - * Global getter method - * - * @param string $name name of the property we want to retrieve - * - * @return mixed the called property - */ - public function __get(string $name): mixed - { - if ($name == self::FIELD) { - return parent::__get('value'); - } else { - return parent::__get($name); - } - } /** * Get localized count string for object list diff --git a/lib/GaletteAuto/Controllers/Controller.php b/lib/GaletteAuto/Controllers/Controller.php index f9d8a6e..d8c65ee 100644 --- a/lib/GaletteAuto/Controllers/Controller.php +++ b/lib/GaletteAuto/Controllers/Controller.php @@ -369,7 +369,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->getId()), 'brands' => $auto->model->brand->getList(), 'colors' => $auto->color->getList(), 'bodies' => $auto->body->getList(), diff --git a/lib/GaletteAuto/Controllers/Crud/PropertiesController.php b/lib/GaletteAuto/Controllers/Crud/PropertiesController.php index 279dba5..7cf6b79 100644 --- a/lib/GaletteAuto/Controllers/Crud/PropertiesController.php +++ b/lib/GaletteAuto/Controllers/Crud/PropertiesController.php @@ -281,7 +281,7 @@ public function propertyEdit(Response $response, string $property, ?int $id = nu $object->load($id); $title = str_replace( '%s', - $object->{$object::FIELD}, + $object->getValue(), _T("Change '%s'", "auto") ); } @@ -348,11 +348,11 @@ public function doPropertyEdit( = _T("- An error occurred while saving record. Please try again.", "auto"); } - $value = $post[$object->field] ?? null; + $value = $post[$object->getField()] ?? null; if ($value == null) { $error_detected[] = _T("- You must provide a value!", "auto"); } else { - $object->value = $value; + $object->setValue($value); } if (count($error_detected) == 0) { @@ -418,7 +418,7 @@ public function propertyShow(Response $response, string $property, int $id): Res $object->load($id); $title = str_replace( '%s', - $object->{$object::FIELD}, + $object->getValue(), _T("Show '%s' brand", "auto") ); @@ -434,7 +434,7 @@ public function propertyShow(Response $response, string $property, int $id): Res $this->login, new ModelsList() ); - $params['models'] = $models->getList($object->id); + $params['models'] = $models->getList($object->getId()); } // display page @@ -476,11 +476,11 @@ public function removeProperty(Request $request, Response $response, string $pro 'page_title' => sprintf( _T('Remove %1$s %2$s', 'auto'), $object->getFieldLabel(), - $object->value + $object->getValue() ), 'form_url' => $this->routeparser->urlFor( 'doRemoveProperty', - ['property' => $property, 'id' => $object->id] + ['property' => $property, 'id' => $object->getId()] ), 'cancel_uri' => $route, 'data' => $data diff --git a/lib/GaletteAuto/Finition.php b/lib/GaletteAuto/Finition.php index 20e8262..0341dce 100644 --- a/lib/GaletteAuto/Finition.php +++ b/lib/GaletteAuto/Finition.php @@ -58,21 +58,6 @@ public function getRouteName(): string return 'finition'; } - /** - * Global getter method - * - * @param string $name name of the property we want to retrieve - * - * @return mixed the called property - */ - public function __get(string $name): mixed - { - if ($name == self::FIELD) { - return parent::__get('value'); - } else { - return parent::__get($name); - } - } /** * Get localized count string for object list diff --git a/lib/GaletteAuto/History.php b/lib/GaletteAuto/History.php index 3227a4d..9a4e4ec 100644 --- a/lib/GaletteAuto/History.php +++ b/lib/GaletteAuto/History.php @@ -144,11 +144,11 @@ private function formatEntries(array $entries): void //associate color $color = new Color($this->zdb, (int)$entry['id_color']); - $entry['color'] = $color->value; + $entry['color'] = $color->getValue(); //associate state $state = new State($this->zdb, (int)$entry['id_state']); - $entry['state'] = $state->value; + $entry['state'] = $state->getValue(); $this->entries[] = $entry; } diff --git a/lib/GaletteAuto/Model.php b/lib/GaletteAuto/Model.php index e00f2f2..31928d6 100644 --- a/lib/GaletteAuto/Model.php +++ b/lib/GaletteAuto/Model.php @@ -110,7 +110,7 @@ public function store(bool $new = false): bool try { $values = [ 'model' => $this->model, - Brand::PK => $this->brand->id + Brand::PK => $this->brand->getId() ]; if ($new) { $insert = $this->zdb->insert(AUTO_PREFIX . self::TABLE); diff --git a/lib/GaletteAuto/State.php b/lib/GaletteAuto/State.php index eee234e..b2e81d7 100644 --- a/lib/GaletteAuto/State.php +++ b/lib/GaletteAuto/State.php @@ -58,21 +58,6 @@ public function getRouteName(): string return 'state'; } - /** - * Global getter method - * - * @param string $name name of the property we want to retrieve - * - * @return mixed the called property - */ - public function __get(string $name): mixed - { - if ($name == self::FIELD) { - return parent::__get('value'); - } else { - return parent::__get($name); - } - } /** * Get localized count string for object list diff --git a/lib/GaletteAuto/Transmission.php b/lib/GaletteAuto/Transmission.php index b795f07..74047f2 100644 --- a/lib/GaletteAuto/Transmission.php +++ b/lib/GaletteAuto/Transmission.php @@ -58,21 +58,6 @@ public function getRouteName(): string return 'transmission'; } - /** - * Global getter method - * - * @param string $name name of the property we want to retrieve - * - * @return mixed the called property - */ - public function __get(string $name): mixed - { - if ($name == self::FIELD) { - return parent::__get('value'); - } else { - return parent::__get($name); - } - } /** * Get localized count string for object list diff --git a/templates/default/model.html.twig b/templates/default/model.html.twig index 62f487d..04424dd 100644 --- a/templates/default/model.html.twig +++ b/templates/default/model.html.twig @@ -30,7 +30,7 @@ {% if brands|length > 0 %} {% include "components/forms/select.html.twig" with { id: 'brand', - value: model.brand.id, + value: model.brand.getId(), values: brand_list_values, label: _T("Brand", "auto"), required: true diff --git a/templates/default/models_list.html.twig b/templates/default/models_list.html.twig index 0023903..fddfa73 100644 --- a/templates/default/models_list.html.twig +++ b/templates/default/models_list.html.twig @@ -41,18 +41,18 @@ {% set edit_link = url_for("modelEdit", {"id": m.id}) %} {{ m.model }} - {{ m.brand.value }} + {{ m.brand.getValue() }} - + - + diff --git a/templates/default/object.html.twig b/templates/default/object.html.twig index 296797c..5a4a107 100644 --- a/templates/default/object.html.twig +++ b/templates/default/object.html.twig @@ -7,11 +7,10 @@ {% extends 'page.html.twig' %} {% block content %} - {% set name = obj.name %} - {% set pk = obj.pk %} - {% set field = obj.field %} + {% set pk = obj.getPk() %} + {% set field = obj.getField() %} -
+
@@ -21,7 +20,7 @@ {% include "components/forms/text.html.twig" with { id: field, name: field, - value: obj.value, + value: obj.getValue(), label: obj.getFieldLabel(), required: true, autofocus: true @@ -35,7 +34,7 @@ - +
{% endblock %} diff --git a/templates/default/object_list.html.twig b/templates/default/object_list.html.twig index cd494d5..419285f 100644 --- a/templates/default/object_list.html.twig +++ b/templates/default/object_list.html.twig @@ -6,9 +6,8 @@ {% extends 'elements/list.html.twig' %} -{% set name = obj.name %} -{% set pk = obj.pk %} -{% set field = obj.field %} +{% set pk = obj.getPk() %} +{% set field = obj.getField() %} {% set nb = list|length %} diff --git a/templates/default/object_show.html.twig b/templates/default/object_show.html.twig index a4a0aea..3f2e8d8 100644 --- a/templates/default/object_show.html.twig +++ b/templates/default/object_show.html.twig @@ -7,9 +7,8 @@ {% extends 'page.html.twig' %} {% block content %} - {% set name = obj.name %} - {% set pk = obj.pk %} - {% set field = obj.field %} + {% set pk = obj.getPk() %} + {% set field = obj.getField() %}
@@ -22,7 +21,7 @@ {% if models|length > 0 %}

- {{ _T("Registered models for the brand '%s'", "auto")|replace({'%s': obj.value}) }} + {{ _T("Registered models for the brand '%s'", "auto")|replace({'%s': obj.getValue()}) }}

@@ -39,7 +38,7 @@
{% else %}
-

{{ _T("The brand '%s' does not have any registered model at this time.", "auto")|replace({'%s': obj.value}) }}

+

{{ _T("The brand '%s' does not have any registered model at this time.", "auto")|replace({'%s': obj.getValue()}) }}

{% endif %}
diff --git a/templates/default/public_vehicles_list.html.twig b/templates/default/public_vehicles_list.html.twig index 19080cc..79db1b2 100644 --- a/templates/default/public_vehicles_list.html.twig +++ b/templates/default/public_vehicles_list.html.twig @@ -64,11 +64,11 @@ {# members kept out of the members list are not named #} {{ public_owners[auto.id] ? auto.owner.sfullname : '—' }} - {{ brand.value }} + {{ brand.getValue() }} {{ auto.model.model }} {# what the car is, then how it is built #} - {% set identity = [auto.getFirstCirculationYear(), auto.body.value, auto.finition.value]|filter(v => v) %} + {% set identity = [auto.getFirstCirculationYear(), auto.body.getValue(), auto.finition.getValue()]|filter(v => v) %} {% set specs = [] %} {% if auto.engine_size %} {% set specs = specs|merge([_T("%1$s cc", "auto")|replace({'%1$s': auto.engine_size|format_number({}, 'decimal', 'default', i18n.getWebID())})]) %} @@ -76,7 +76,7 @@ {% if auto.horsepower %} {% set specs = specs|merge([_T("%1$s hp", "auto")|replace({'%1$s': auto.horsepower|format_number({}, 'decimal', 'default', i18n.getWebID())})]) %} {% endif %} - {% set specs = specs|merge([auto.getFuelLabel(), auto.transmission.value, auto.color.value]|filter(v => v)) %} + {% set specs = specs|merge([auto.getFuelLabel(), auto.transmission.getValue(), auto.color.getValue()]|filter(v => v)) %} {% if identity|length > 0 %} {{ identity|join(' · ') }} {% endif %} diff --git a/templates/default/vehicles.html.twig b/templates/default/vehicles.html.twig index 85ad259..50f0af9 100644 --- a/templates/default/vehicles.html.twig +++ b/templates/default/vehicles.html.twig @@ -33,7 +33,7 @@ {% include "components/forms/select.html.twig" with { id: 'brand', - value: car.model.brand.id, + value: car.model.brand.getId(), values: brand_list_values, label: _T("Brand", "auto"), required: required.brand is defined @@ -162,7 +162,7 @@ {% include "components/forms/select.html.twig" with { id: 'color', - value: car.color.id, + value: car.color.getId(), values: color_list_values, label: _T("Color", "auto"), required: required.color is defined @@ -175,7 +175,7 @@ {% include "components/forms/select.html.twig" with { id: 'state', - value: car.state.id, + value: car.state.getId(), values: state_list_values, label: _T("State", "auto"), required: required.state is defined @@ -204,7 +204,7 @@ {% include "components/forms/select.html.twig" with { id: 'body', - value: car.body.id, + value: car.body.getId(), values: body_list_values, label: _T("Body", "auto"), required: required.body is defined @@ -217,7 +217,7 @@ {% include "components/forms/select.html.twig" with { id: 'transmission', - value: car.transmission.id, + value: car.transmission.getId(), values: transmission_list_values, label: _T("Transmission", "auto"), required: required.transmission is defined @@ -230,7 +230,7 @@ {% include "components/forms/select.html.twig" with { id: 'finition', - value: car.finition.id, + value: car.finition.getId(), values: finition_list_values, label: _T("Finition", "auto"), required: required.finition is defined diff --git a/templates/default/vehicles_list.html.twig b/templates/default/vehicles_list.html.twig index 3e0daad..b6ad7b6 100644 --- a/templates/default/vehicles_list.html.twig +++ b/templates/default/vehicles_list.html.twig @@ -63,7 +63,7 @@ {{ auto.name }} {{ auto.owner.sfullname }} - {{ brand.value }} + {{ brand.getValue() }} {{ auto.model.model }} {% if login.isAdmin() or login.isStaff() or auto.owner.id == login.id or login.isGroupManager() and preferences.pref_bool_groupsmanagers_edit_member %} diff --git a/tests/GaletteAuto/Controllers/tests/units/Controller.php b/tests/GaletteAuto/Controllers/tests/units/Controller.php index e86fe2a..2edf153 100644 --- a/tests/GaletteAuto/Controllers/tests/units/Controller.php +++ b/tests/GaletteAuto/Controllers/tests/units/Controller.php @@ -46,9 +46,9 @@ public function setUp(): void foreach ($values as $property => $value) { $class = '\GaletteAuto\\' . ucfirst($property); $object = new $class($this->zdb); - $object->value = $value; + $object->setValue($value); $this->assertTrue($object->store(true)); - $this->props[$property] = $object->id; + $this->props[$property] = $object->getId(); } $model = new \GaletteAuto\Model($this->zdb); @@ -993,14 +993,14 @@ public function testAjaxModels(): void { $model = new \GaletteAuto\Model($this->zdb); $brand = new \GaletteAuto\Brand($this->zdb); - $brand->value = 'Renault'; + $brand->setValue('Renault'); $this->assertTrue($brand->store(true)); - $this->assertTrue($model->check(['model' => 'Clio', 'brand' => $brand->id])); + $this->assertTrue($model->check(['model' => 'Clio', 'brand' => $brand->getId()])); $this->assertTrue($model->store(true)); $this->getMemberOne(); $this->logMember($this->dataAdherentOne()); - $request = $this->createRequest('ajaxModels', [], 'POST')->withParsedBody(['brand' => (string)$brand->id]); + $request = $this->createRequest('ajaxModels', [], 'POST')->withParsedBody(['brand' => (string)$brand->getId()]); $test_response = $this->app->handle($request); $this->assertSame(200, $test_response->getStatusCode()); $models = json_decode((string)$test_response->getBody(), true); diff --git a/tests/GaletteAuto/Controllers/tests/units/ModelsController.php b/tests/GaletteAuto/Controllers/tests/units/ModelsController.php index a84f1e2..5ef791c 100644 --- a/tests/GaletteAuto/Controllers/tests/units/ModelsController.php +++ b/tests/GaletteAuto/Controllers/tests/units/ModelsController.php @@ -33,9 +33,9 @@ public function setUp(): void { parent::setUp(); $brand = new Brand($this->zdb); - $brand->value = 'Peugeot'; + $brand->setValue('Peugeot'); $this->assertTrue($brand->store(true)); - $this->brand_id = $brand->id; + $this->brand_id = $brand->getId(); } /** @@ -128,9 +128,9 @@ private function createVehicle(int $model_id): void foreach (['Body', 'Color', 'Finition', 'State', 'Transmission'] as $property) { $class = '\\GaletteAuto\\' . $property; $object = new $class($this->zdb); - $object->value = 'Test ' . $property; + $object->setValue('Test ' . $property); $this->assertTrue($object->store(true)); - $values[$class::PK] = $object->id; + $values[$class::PK] = $object->getId(); } $insert = $this->zdb->insert(AUTO_PREFIX . \GaletteAuto\Auto::TABLE); $insert->values($values + [ diff --git a/tests/GaletteAuto/Controllers/tests/units/PropertiesController.php b/tests/GaletteAuto/Controllers/tests/units/PropertiesController.php index 4eb4637..7275f35 100644 --- a/tests/GaletteAuto/Controllers/tests/units/PropertiesController.php +++ b/tests/GaletteAuto/Controllers/tests/units/PropertiesController.php @@ -40,9 +40,9 @@ public function tearDown(): void private function createColor(string $value): int { $color = new Color($this->zdb); - $color->value = $value; + $color->setValue($value); $this->assertTrue($color->store(true)); - return $color->id; + return $color->getId(); } /** @@ -54,7 +54,7 @@ private function getColor(int $id): string { $color = new Color($this->zdb); $this->assertTrue($color->load($id)); - return $color->value; + return $color->getValue(); } /** @@ -125,7 +125,7 @@ public function testLists(): void $class = \GaletteAuto\AbstractObject::getClassForPropName($property); foreach (['Zeta ' . $property, 'Alpha ' . $property] as $value) { $object = new $class($this->zdb); - $object->value = $value; + $object->setValue($value); $this->assertTrue($object->store(true)); } @@ -160,10 +160,10 @@ public function testEditAndShow(): void { $id = $this->createColor('Red'); $brand = new \GaletteAuto\Brand($this->zdb); - $brand->value = 'Peugeot'; + $brand->setValue('Peugeot'); $this->assertTrue($brand->store(true)); $model = new \GaletteAuto\Model($this->zdb); - $this->assertTrue($model->check(['model' => '307', 'brand' => $brand->id])); + $this->assertTrue($model->check(['model' => '307', 'brand' => $brand->getId()])); $this->assertTrue($model->store(true)); $this->logSuperAdmin(); @@ -174,7 +174,7 @@ public function testEditAndShow(): void $this->assertStringContainsString('value="Red"', (string)$test_response->getBody()); $test_response = $this->app->handle( - $this->createRequest('propertyShow', ['property' => 'brand', 'id' => (string)$brand->id]) + $this->createRequest('propertyShow', ['property' => 'brand', 'id' => (string)$brand->getId()]) ); $this->expectOK($test_response); $body = (string)$test_response->getBody(); @@ -214,15 +214,15 @@ public function testRemove(): void foreach (['Body', 'Finition', 'State', 'Transmission'] as $property) { $class = '\\GaletteAuto\\' . $property; $object = new $class($this->zdb); - $object->value = 'Test ' . $property; + $object->setValue('Test ' . $property); $this->assertTrue($object->store(true)); - $values[$class::PK] = $object->id; + $values[$class::PK] = $object->getId(); } $brand = new \GaletteAuto\Brand($this->zdb); - $brand->value = 'Peugeot'; + $brand->setValue('Peugeot'); $this->assertTrue($brand->store(true)); $model = new \GaletteAuto\Model($this->zdb); - $this->assertTrue($model->check(['model' => '307', 'brand' => $brand->id])); + $this->assertTrue($model->check(['model' => '307', 'brand' => $brand->getId()])); $this->assertTrue($model->store(true)); $insert = $this->zdb->insert(AUTO_PREFIX . \GaletteAuto\Auto::TABLE); $insert->values($values + [ diff --git a/tests/GaletteAuto/tests/units/Auto.php b/tests/GaletteAuto/tests/units/Auto.php index 1a449cc..8c5cbe7 100644 --- a/tests/GaletteAuto/tests/units/Auto.php +++ b/tests/GaletteAuto/tests/units/Auto.php @@ -38,34 +38,34 @@ public function setUp(): void public function testCrud(): void { $body = new \GaletteAuto\Body($this->zdb); - $body->value = 'Berline'; + $body->setValue('Berline'); $this->assertTrue($body->store(true)); - $body_id = $body->id; + $body_id = $body->getId(); $color = new \GaletteAuto\Color($this->zdb); - $color->value = 'Grey'; + $color->setValue('Grey'); $this->assertTrue($color->store(true)); - $color_id = $color->id; + $color_id = $color->getId(); $finition = new \GaletteAuto\Finition($this->zdb); - $finition->value = 'Standard'; + $finition->setValue('Standard'); $this->assertTrue($finition->store(true)); - $finition_id = $finition->id; + $finition_id = $finition->getId(); $state = new \GaletteAuto\State($this->zdb); - $state->value = 'Correct'; + $state->setValue('Correct'); $this->assertTrue($state->store(true)); - $state_id = $state->id; + $state_id = $state->getId(); $transmission = new \GaletteAuto\Transmission($this->zdb); - $transmission->value = 'Manual'; + $transmission->setValue('Manual'); $this->assertTrue($transmission->store(true)); - $transmission_id = $transmission->id; + $transmission_id = $transmission->getId(); $brand = new \GaletteAuto\Brand($this->zdb); - $brand->value = 'Peugeot'; + $brand->setValue('Peugeot'); $this->assertTrue($brand->store(true)); - $brand_id = $brand->id; + $brand_id = $brand->getId(); $model = new \GaletteAuto\Model($this->zdb); $data = [ @@ -181,9 +181,9 @@ public function testCrud(): void $adh2 = $this->getMemberTwo(); $color2 = new \GaletteAuto\Color($this->zdb); - $color2->value = 'Yellow'; + $color2->setValue('Yellow'); $this->assertTrue($color2->store(true)); - $color2_id = $color2->id; + $color2_id = $color2->getId(); $data = [ 'registration' => 'GA-123-TE', diff --git a/tests/GaletteAuto/tests/units/Body.php b/tests/GaletteAuto/tests/units/Body.php index 1df13ba..d0c57e5 100644 --- a/tests/GaletteAuto/tests/units/Body.php +++ b/tests/GaletteAuto/tests/units/Body.php @@ -43,9 +43,9 @@ public function testCrud(): void $this->assertCount(0, $body->getList()); //Add new body - $body->value = 'Coupe'; + $body->setValue('Coupe'); $this->assertTrue($body->store(true)); - $first_id = $body->id; + $first_id = $body->getId(); $this->assertCount(1, $body->getList()); $listed_body = $body->getList()[0]; @@ -56,16 +56,16 @@ public function testCrud(): void //add another one $body = new \GaletteAuto\Body($this->zdb); - $body->value = 'Brea'; + $body->setValue('Brea'); $this->assertTrue($body->store(true)); - $id = $body->id; + $id = $body->getId(); $this->assertCount(2, $body->getList()); $this->assertSame('2 bodies', $body->displayCount()); $body = new \GaletteAuto\Body($this->zdb); $this->assertTrue($body->load($id)); - $body->value = 'Break'; + $body->setValue('Break'); $this->assertTrue($body->store()); $this->assertCount(2, $body->getList()); diff --git a/tests/GaletteAuto/tests/units/Brand.php b/tests/GaletteAuto/tests/units/Brand.php index 11af488..89c55a8 100644 --- a/tests/GaletteAuto/tests/units/Brand.php +++ b/tests/GaletteAuto/tests/units/Brand.php @@ -43,9 +43,9 @@ public function testCrud(): void $this->assertCount(0, $brand->getList()); //Add new brand - $brand->value = 'Audi'; + $brand->setValue('Audi'); $this->assertTrue($brand->store(true)); - $first_id = $brand->id; + $first_id = $brand->getId(); $this->assertCount(1, $brand->getList()); $listed_brand = $brand->getList()[0]; @@ -56,16 +56,16 @@ public function testCrud(): void //add another one $brand = new \GaletteAuto\Brand($this->zdb); - $brand->value = 'Mercede'; + $brand->setValue('Mercede'); $this->assertTrue($brand->store(true)); - $id = $brand->id; + $id = $brand->getId(); $this->assertCount(2, $brand->getList()); $this->assertSame('2 brands', $brand->displayCount()); $brand = new \GaletteAuto\Brand($this->zdb); $this->assertTrue($brand->load($id)); - $brand->value = 'Mercedes'; + $brand->setValue('Mercedes'); $this->assertTrue($brand->store()); $this->assertCount(2, $brand->getList()); diff --git a/tests/GaletteAuto/tests/units/Color.php b/tests/GaletteAuto/tests/units/Color.php index 2018558..e825f08 100644 --- a/tests/GaletteAuto/tests/units/Color.php +++ b/tests/GaletteAuto/tests/units/Color.php @@ -43,9 +43,9 @@ public function testCrud(): void $this->assertCount(0, $color->getList()); //Add new color - $color->value = 'Red'; + $color->setValue('Red'); $this->assertTrue($color->store(true)); - $first_id = $color->id; + $first_id = $color->getId(); $this->assertCount(1, $color->getList()); $listed_color = $color->getList()[0]; @@ -56,9 +56,9 @@ public function testCrud(): void //add another one $color = new \GaletteAuto\Color($this->zdb); - $color->value = 'Blu'; + $color->setValue('Blu'); $this->assertTrue($color->store(true)); - $id = $color->id; + $id = $color->getId(); $this->assertCount(2, $color->getList()); $this->assertSame('2 colors', $color->displayCount()); @@ -67,7 +67,7 @@ public function testCrud(): void $color = new \GaletteAuto\Color($this->zdb); $this->assertTrue($color->load($id)); - $color->value = 'Blue'; + $color->setValue('Blue'); $this->assertTrue($color->store()); $this->assertCount(2, $color->getList()); diff --git a/tests/GaletteAuto/tests/units/Finition.php b/tests/GaletteAuto/tests/units/Finition.php index 5229714..55f59e3 100644 --- a/tests/GaletteAuto/tests/units/Finition.php +++ b/tests/GaletteAuto/tests/units/Finition.php @@ -43,9 +43,9 @@ public function testCrud(): void $this->assertCount(0, $finition->getList()); //Add new finition - $finition->value = 'Feline'; + $finition->setValue('Feline'); $this->assertTrue($finition->store(true)); - $first_id = $finition->id; + $first_id = $finition->getId(); $this->assertCount(1, $finition->getList()); $listed_finition = $finition->getList()[0]; @@ -56,16 +56,16 @@ public function testCrud(): void //add another one $finition = new \GaletteAuto\Finition($this->zdb); - $finition->value = 'R'; + $finition->setValue('R'); $this->assertTrue($finition->store(true)); - $id = $finition->id; + $id = $finition->getId(); $this->assertCount(2, $finition->getList()); $this->assertSame('2 finitions', $finition->displayCount()); $finition = new \GaletteAuto\Finition($this->zdb); $this->assertTrue($finition->load($id)); - $finition->value = 'RS'; + $finition->setValue('RS'); $this->assertTrue($finition->store()); $this->assertCount(2, $finition->getList()); diff --git a/tests/GaletteAuto/tests/units/Model.php b/tests/GaletteAuto/tests/units/Model.php index cd4a805..47d5932 100644 --- a/tests/GaletteAuto/tests/units/Model.php +++ b/tests/GaletteAuto/tests/units/Model.php @@ -28,15 +28,15 @@ public function testCrud(): void { $brand = new \GaletteAuto\Brand($this->zdb); //Add new brand - $brand->value = 'Audi'; + $brand->setValue('Audi'); $this->assertTrue($brand->store(true)); - $first_brand_id = $brand->id; + $first_brand_id = $brand->getId(); //add another brand $brand = new \GaletteAuto\Brand($this->zdb); - $brand->value = 'Mercedes'; + $brand->setValue('Mercedes'); $this->assertTrue($brand->store(true)); - $second_brand_id = $brand->id; + $second_brand_id = $brand->getId(); $this->assertCount(2, $brand->getList()); diff --git a/tests/GaletteAuto/tests/units/State.php b/tests/GaletteAuto/tests/units/State.php index c554027..93b5447 100644 --- a/tests/GaletteAuto/tests/units/State.php +++ b/tests/GaletteAuto/tests/units/State.php @@ -43,9 +43,9 @@ public function testCrud(): void $this->assertCount(0, $state->getList()); //Add new state - $state->value = 'Good'; + $state->setValue('Good'); $this->assertTrue($state->store(true)); - $first_id = $state->id; + $first_id = $state->getId(); $this->assertCount(1, $state->getList()); $listed_state = $state->getList()[0]; @@ -56,16 +56,16 @@ public function testCrud(): void //add another one $state = new \GaletteAuto\State($this->zdb); - $state->value = 'Wrec'; + $state->setValue('Wrec'); $this->assertTrue($state->store(true)); - $id = $state->id; + $id = $state->getId(); $this->assertCount(2, $state->getList()); $this->assertSame('2 states', $state->displayCount()); $state = new \GaletteAuto\State($this->zdb); $this->assertTrue($state->load($id)); - $state->value = 'Wreck'; + $state->setValue('Wreck'); $this->assertTrue($state->store()); $this->assertCount(2, $state->getList()); diff --git a/tests/GaletteAuto/tests/units/Transmission.php b/tests/GaletteAuto/tests/units/Transmission.php index 7d4041b..8ec67c2 100644 --- a/tests/GaletteAuto/tests/units/Transmission.php +++ b/tests/GaletteAuto/tests/units/Transmission.php @@ -43,9 +43,9 @@ public function testCrud(): void $this->assertCount(0, $transmission->getList()); //Add new transmission - $transmission->value = 'Manual'; + $transmission->setValue('Manual'); $this->assertTrue($transmission->store(true)); - $first_id = $transmission->id; + $first_id = $transmission->getId(); $this->assertCount(1, $transmission->getList()); $listed_transmission = $transmission->getList()[0]; @@ -56,16 +56,16 @@ public function testCrud(): void //add another one $transmission = new \GaletteAuto\Transmission($this->zdb); - $transmission->value = 'Auto'; + $transmission->setValue('Auto'); $this->assertTrue($transmission->store(true)); - $id = $transmission->id; + $id = $transmission->getId(); $this->assertCount(2, $transmission->getList()); $this->assertSame('2 transmissions', $transmission->displayCount()); $transmission = new \GaletteAuto\Transmission($this->zdb); $this->assertTrue($transmission->load($id)); - $transmission->value = 'Automatic'; + $transmission->setValue('Automatic'); $this->assertTrue($transmission->store()); $this->assertCount(2, $transmission->getList()); From 5b909afda2771e19ca77a08712eac0d1918d1047 Mon Sep 17 00:00:00 2001 From: Johan Cwiklinski Date: Sat, 26 Sep 2026 10:53:43 +0200 Subject: [PATCH 2/6] Give models explicit accessors, build their brand from the joined row --- lib/GaletteAuto/Auto.php | 2 +- lib/GaletteAuto/Controllers/Controller.php | 4 +- .../Controllers/Crud/ModelsController.php | 4 +- lib/GaletteAuto/Model.php | 46 ++++++++++--------- .../Controllers/tests/units/Controller.php | 2 +- .../tests/units/ModelsController.php | 8 ++-- .../tests/units/PropertiesController.php | 2 +- tests/GaletteAuto/tests/units/Auto.php | 2 +- tests/GaletteAuto/tests/units/Model.php | 2 +- 9 files changed, 37 insertions(+), 35 deletions(-) diff --git a/lib/GaletteAuto/Auto.php b/lib/GaletteAuto/Auto.php index 6c5cbc8..51e089f 100644 --- a/lib/GaletteAuto/Auto.php +++ b/lib/GaletteAuto/Auto.php @@ -319,7 +319,7 @@ public function store(bool $new = false): bool $values[$k] = $this->finition->getId(); break; case Model::PK: - $values[$k] = $this->model->id; + $values[$k] = $this->model->getId(); break; case Adherent::PK: $values[$k] = $this->owner->id; diff --git a/lib/GaletteAuto/Controllers/Controller.php b/lib/GaletteAuto/Controllers/Controller.php index d8c65ee..1c2250a 100644 --- a/lib/GaletteAuto/Controllers/Controller.php +++ b/lib/GaletteAuto/Controllers/Controller.php @@ -369,8 +369,8 @@ public function showAddEditVehicle(Request $request, Response $response, string 'require_calendar' => true, 'require_dialog' => true, 'car' => $auto, - 'models' => $models->getList($auto->model->brand->getId()), - 'brands' => $auto->model->brand->getList(), + 'models' => $models->getList($auto->model->getBrand()->getId()), + 'brands' => $auto->model->getBrand()->getList(), 'colors' => $auto->color->getList(), 'bodies' => $auto->body->getList(), 'transmissions' => $auto->transmission->getList(), diff --git a/lib/GaletteAuto/Controllers/Crud/ModelsController.php b/lib/GaletteAuto/Controllers/Crud/ModelsController.php index 0bb9332..721721e 100644 --- a/lib/GaletteAuto/Controllers/Crud/ModelsController.php +++ b/lib/GaletteAuto/Controllers/Crud/ModelsController.php @@ -182,7 +182,7 @@ public function edit(Request $request, Response $response, ?int $id = null, stri if ($action === 'edit') { $title = sprintf( _T("Change model '%s'", "auto"), - $model->model + $model->getModel() ); } else { $title = _T("New model", "auto"); @@ -322,7 +322,7 @@ public function confirmRemoveTitle(array $args): string return sprintf( //TRANS: first parameter is the model name _T('Remove model "%1$s"', 'auto'), - $model->model + $model->getModel() ); } } diff --git a/lib/GaletteAuto/Model.php b/lib/GaletteAuto/Model.php index 31928d6..482c4cf 100644 --- a/lib/GaletteAuto/Model.php +++ b/lib/GaletteAuto/Model.php @@ -19,10 +19,6 @@ * Automobile Models class for galette Auto plugin * * @author Johan Cwiklinski - * - * @property int $id - * @property string $model - * @property Brand $brand */ class Model { @@ -30,12 +26,12 @@ class Model public const string PK = 'id_model'; public const string FIELD = 'model'; - protected int $id; - protected string $model; + protected ?int $id = null; + protected ?string $model = null; protected Brand $brand; /** @var string[] */ - private array $errors; + private array $errors = []; private Db $zdb; /** @@ -96,8 +92,13 @@ public function load(int $id): bool private function loadFromRS(ArrayObject $r): void { $this->id = (int)$r[self::PK]; - $this->model = (string)$r['model']; - $this->brand->load((int)$r[Brand::PK]); + $this->model = (string)$r[self::FIELD]; + if (isset($r[Brand::FIELD])) { + //brand has been joined + $this->brand->loadFromRow($r); + } else { + $this->brand->load((int)$r[Brand::PK]); + } } /** @@ -166,26 +167,27 @@ public function delete(array $ids): bool } /** - * Global getter method - * - * @param string $name name of the property we want to retrieve - * - * @return mixed the called property + * Get model ID */ - public function __get(string $name): mixed + public function getId(): ?int { - return $this->$name ?? null; + return $this->id; } /** - * Global isset method - * Required for twig to access properties via __get - * - * @param string $name name of the property we want to retrieve + * Get model name + */ + public function getModel(): ?string + { + return $this->model; + } + + /** + * Get model brand */ - public function __isset(string $name): bool + public function getBrand(): Brand { - return property_exists($this, $name); + return $this->brand; } /** diff --git a/tests/GaletteAuto/Controllers/tests/units/Controller.php b/tests/GaletteAuto/Controllers/tests/units/Controller.php index 2edf153..6dfafd0 100644 --- a/tests/GaletteAuto/Controllers/tests/units/Controller.php +++ b/tests/GaletteAuto/Controllers/tests/units/Controller.php @@ -54,7 +54,7 @@ public function setUp(): void $model = new \GaletteAuto\Model($this->zdb); $this->assertTrue($model->check(['model' => '307', 'brand' => $this->props['brand']])); $this->assertTrue($model->store(true)); - $this->props['model'] = $model->id; + $this->props['model'] = $model->getId(); } /** diff --git a/tests/GaletteAuto/Controllers/tests/units/ModelsController.php b/tests/GaletteAuto/Controllers/tests/units/ModelsController.php index 5ef791c..b1a9ed6 100644 --- a/tests/GaletteAuto/Controllers/tests/units/ModelsController.php +++ b/tests/GaletteAuto/Controllers/tests/units/ModelsController.php @@ -57,7 +57,7 @@ private function createModel(string $name): int $model = new Model($this->zdb); $this->assertTrue($model->check(['model' => $name, 'brand' => $this->brand_id])); $this->assertTrue($model->store(true)); - return $model->id; + return $model->getId(); } /** @@ -77,8 +77,8 @@ public function testEditUsesRouteId(): void $test_response->getHeaders() ); $this->expectFlashData(['success_detected' => ['Model has been saved!']]); - $this->assertSame('306', (new Model($this->zdb, $first))->model); - $this->assertSame('308', (new Model($this->zdb, $second))->model); + $this->assertSame('306', (new Model($this->zdb, $first))->getModel()); + $this->assertSame('308', (new Model($this->zdb, $second))->getModel()); } /** @@ -102,7 +102,7 @@ public function testEditErrorKeepsPostedValues(): void $test_response = $this->app->handle($request); $this->assertSame(200, $test_response->getStatusCode()); $this->assertStringContainsString('value="Posted model"', (string)$test_response->getBody()); - $this->assertSame('307', (new Model($this->zdb, $id))->model); + $this->assertSame('307', (new Model($this->zdb, $id))->getModel()); } /** diff --git a/tests/GaletteAuto/Controllers/tests/units/PropertiesController.php b/tests/GaletteAuto/Controllers/tests/units/PropertiesController.php index 7275f35..f05e1e3 100644 --- a/tests/GaletteAuto/Controllers/tests/units/PropertiesController.php +++ b/tests/GaletteAuto/Controllers/tests/units/PropertiesController.php @@ -231,7 +231,7 @@ public function testRemove(): void 'car_first_registration_date' => '2001-02-12', 'car_first_circulation_date' => '2001-02-13', 'car_creation_date' => date('Y-m-d'), - \GaletteAuto\Model::PK => $model->id, + \GaletteAuto\Model::PK => $model->getId(), \Galette\Entity\Adherent::PK => $this->getMemberOne()->id, ]); $this->zdb->execute($insert); diff --git a/tests/GaletteAuto/tests/units/Auto.php b/tests/GaletteAuto/tests/units/Auto.php index 8c5cbe7..951913c 100644 --- a/tests/GaletteAuto/tests/units/Auto.php +++ b/tests/GaletteAuto/tests/units/Auto.php @@ -74,7 +74,7 @@ public function testCrud(): void ]; $this->assertTrue($model->check($data)); $this->assertTrue($model->store(true)); - $model_id = $model->id; + $model_id = $model->getId(); $this->logSuperAdmin(); $access = new \GaletteAuto\VehicleAccess($this->zdb, $this->login, $this->preferences); diff --git a/tests/GaletteAuto/tests/units/Model.php b/tests/GaletteAuto/tests/units/Model.php index 47d5932..d43d079 100644 --- a/tests/GaletteAuto/tests/units/Model.php +++ b/tests/GaletteAuto/tests/units/Model.php @@ -99,7 +99,7 @@ public function testCrud(): void ]; $this->assertTrue($model->check($data)); $this->assertTrue($model->store(true)); - $id_model = $model->id; + $id_model = $model->getId(); $this->assertCount(2, $models->getList()); From dc9bc699f8d72af2dc328edb129187d4ab95b7b4 Mon Sep 17 00:00:00 2001 From: Johan Cwiklinski Date: Sat, 26 Sep 2026 10:59:41 +0200 Subject: [PATCH 3/6] Give vehicle history explicit accessors, join colors and states, load each owner once --- lib/GaletteAuto/Auto.php | 2 +- lib/GaletteAuto/Controllers/Controller.php | 5 +- lib/GaletteAuto/History.php | 124 +++++++++------------ tests/GaletteAuto/tests/units/Auto.php | 6 +- tests/GaletteAuto/tests/units/History.php | 2 +- 5 files changed, 58 insertions(+), 81 deletions(-) diff --git a/lib/GaletteAuto/Auto.php b/lib/GaletteAuto/Auto.php index 51e089f..75011ad 100644 --- a/lib/GaletteAuto/Auto.php +++ b/lib/GaletteAuto/Auto.php @@ -406,7 +406,7 @@ public function store(bool $new = false): bool if ($this->fire_history) { $h_props = []; - foreach ($this->history->fields as $prop) { + foreach ($this->history->getFields() as $prop) { if ($prop != 'history_date') { $h_props[$prop] = $this->$prop; } else { diff --git a/lib/GaletteAuto/Controllers/Controller.php b/lib/GaletteAuto/Controllers/Controller.php index 1c2250a..649f789 100644 --- a/lib/GaletteAuto/Controllers/Controller.php +++ b/lib/GaletteAuto/Controllers/Controller.php @@ -520,16 +520,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); - if (!$auto->load($history->$apk) || !$this->getAccess()->canManageMember($auto->owner_id)) { + if (!$auto->load((int)$history->getCarId()) || !$this->getAccess()->canManageMember($auto->owner_id)) { return $this->accessDenied($response, 'Trying to show history of vehicle #' . $id); } $params = [ 'entries' => $history->getEntries(), - 'page_title' => str_replace('%d', (string)$history->$apk, _T("History of car #%d", "auto")), + 'page_title' => str_replace('%d', (string)$history->getCarId(), _T("History of car #%d", "auto")), 'mode' => $this->isAjax($request) ? 'ajax' : '' ]; diff --git a/lib/GaletteAuto/History.php b/lib/GaletteAuto/History.php index 9a4e4ec..8b85eeb 100644 --- a/lib/GaletteAuto/History.php +++ b/lib/GaletteAuto/History.php @@ -19,36 +19,34 @@ * Automobile History class for galette Auto plugin * * @author Johan Cwiklinski - * - * @property int $id_car - * @property array $fields - * @property array> $entries */ class History { public const string TABLE = 'history'; - private Db $zdb; - /** - * @var array $fields fields list and type + * Tracked fields; any change on one of them adds an entry + * + * @var array */ - private array $fields = [ - Auto::PK => 'integer', - Adherent::PK => 'integer', - 'history_date' => 'datetime', - 'car_registration' => 'text', - Color::PK => 'integer', - State::PK => 'integer' + private const array FIELDS = [ + Auto::PK, + Adherent::PK, + 'history_date', + 'car_registration', + Color::PK, + State::PK ]; + private Db $zdb; + /** * history entries * * @var array> $entries */ - private array $entries; - private int $id_car; + private array $entries = []; + private ?int $id_car = null; /** * Default constructor @@ -74,12 +72,20 @@ public function load(int $id): bool $this->id_car = $id; try { - $select = $this->zdb->select(AUTO_PREFIX . self::TABLE); - $select->where( + $select = $this->zdb->select(AUTO_PREFIX . self::TABLE, 'h'); + $select->join( + ['c' => PREFIX_DB . AUTO_PREFIX . Color::TABLE], + 'h.' . Color::PK . ' = c.' . Color::PK, + [Color::FIELD] + )->join( + ['s' => PREFIX_DB . AUTO_PREFIX . State::TABLE], + 'h.' . State::PK . ' = s.' . State::PK, + [State::FIELD] + )->where( [ - Auto::PK => $id + 'h.' . Auto::PK => $id ] - )->order('history_date ASC'); + )->order('h.history_date ASC'); $results = $this->zdb->execute($select); $this->formatEntries($results->toArray()); @@ -97,7 +103,7 @@ public function load(int $id): bool /** * Get the most recent history entry * - * @return ArrayObject|false row + * @return ArrayObject|false row */ public function getLatest(): ArrayObject|false { @@ -134,21 +140,20 @@ public function getLatest(): ArrayObject|false private function formatEntries(array $entries): void { $this->entries = []; + $owners = []; foreach ($entries as $entry) { //put a formatted date to show $date = new \DateTime($entry['history_date']); $entry['formatted_date'] = $date->format(__('Y-m-d')); - //associate member to current history entry - $entry['owner'] = new Adherent($this->zdb, (int)$entry['id_adh']); - - //associate color - $color = new Color($this->zdb, (int)$entry['id_color']); - $entry['color'] = $color->getValue(); - - //associate state - $state = new State($this->zdb, (int)$entry['id_state']); - $entry['state'] = $state->getValue(); + //associate member to current history entry, once per member + $id_adh = (int)$entry[Adherent::PK]; + if (!isset($owners[$id_adh])) { + $owner = new Adherent($this->zdb); + $owner->disableAllDeps()->load($id_adh); + $owners[$id_adh] = $owner; + } + $entry['owner'] = $owners[$id_adh]; $this->entries[] = $entry; } @@ -161,33 +166,13 @@ private function formatEntries(array $entries): void */ public function register(array $props): void { - Analog::log( - '[' . get_class($this) . '] Trying to register a new history entry.', - Analog::DEBUG - ); - try { - $fields = $this->fields; - ksort($fields); - ksort($props); - - $values = []; - foreach ($props as $key => $prop) { - $values[$key] = $prop; - } - $insert = $this->zdb->insert(AUTO_PREFIX . self::TABLE); - $insert->values($values); + $insert->values(array_intersect_key($props, array_flip(self::FIELDS))); $add = $this->zdb->execute($insert); - if ($add->count() > 0) { - Analog::log( - '[' . get_class($this) - . '] new AutoHistory entry set successfully.', - Analog::DEBUG - ); - } else { - throw new \Exception( + if ($add->count() === 0) { + throw new \RuntimeException( 'An error occurred registering car new history entry :(' ); } @@ -202,28 +187,21 @@ public function register(array $props): void } /** - * Global getter method - * - * @param string $name name of the property we want to retrieve - * - * @return mixed the called property + * Get car ID */ - public function __get(string $name): mixed + public function getCarId(): ?int { - switch ($name) { - case Auto::PK: - return $this->$name; - case 'fields': - return array_keys($this->fields); - } + return $this->id_car; + } - throw new \RuntimeException( - sprintf( - 'Unable to get property "%s::%s"!', - __CLASS__, - $name - ) - ); + /** + * Get tracked fields + * + * @return array + */ + public function getFields(): array + { + return self::FIELDS; } /** diff --git a/tests/GaletteAuto/tests/units/Auto.php b/tests/GaletteAuto/tests/units/Auto.php index 951913c..dfa302a 100644 --- a/tests/GaletteAuto/tests/units/Auto.php +++ b/tests/GaletteAuto/tests/units/Auto.php @@ -161,10 +161,10 @@ public function testCrud(): void 'car_registration', 'id_color', 'id_state', - 'formatted_date', - 'owner', 'color', - 'state' + 'state', + 'formatted_date', + 'owner' ], array_keys($entry) ); diff --git a/tests/GaletteAuto/tests/units/History.php b/tests/GaletteAuto/tests/units/History.php index 46296b8..04bbf9e 100644 --- a/tests/GaletteAuto/tests/units/History.php +++ b/tests/GaletteAuto/tests/units/History.php @@ -27,7 +27,7 @@ class History extends GaletteTestCase public function testGetFields(): void { $history = new \GaletteAuto\History($this->zdb); - $this->assertCount(6, $history->fields); + $this->assertCount(6, $history->getFields()); } /** From e3e742992b303c34c35aef79d1e63b201ca0cfd4 Mon Sep 17 00:00:00 2001 From: Johan Cwiklinski Date: Sat, 26 Sep 2026 14:44:43 +0200 Subject: [PATCH 4/6] Type vehicle properties, replace magic accessors with explicit ones --- lib/GaletteAuto/Auto.php | 742 +++++++++--------- lib/GaletteAuto/Controllers/Controller.php | 44 +- .../default/public_vehicles_list.html.twig | 28 +- templates/default/vehicles.html.twig | 56 +- templates/default/vehicles_list.html.twig | 22 +- tests/GaletteAuto/tests/units/Auto.php | 14 +- 6 files changed, 440 insertions(+), 466 deletions(-) diff --git a/lib/GaletteAuto/Auto.php b/lib/GaletteAuto/Auto.php index 75011ad..435346c 100644 --- a/lib/GaletteAuto/Auto.php +++ b/lib/GaletteAuto/Auto.php @@ -16,70 +16,54 @@ use Galette\Core\Login; use Galette\Core\Plugins; use Galette\Entity\Adherent; -use Laminas\Db\Sql\Expression; use Psr\Http\Message\UploadedFileInterface; /** - * Automobile Transmissions class for galette Auto plugin + * Vehicle entity for galette Auto plugin * * @author Johan Cwiklinski - * - * @property int $id - * @property string $registration - * @property string $name - * @property string $first_registration_date - * @property string $first_circulation_date - * @property int $mileage - * @property string $comment - * @property string $chassis_number - * @property int $seats - * @property int $horsepower - * @property int $engine_size - * @property string $creation_date - * @property ?int $fuel - * @property Color $color - * @property Body $body - * @property State $state - * @property Transmission $transmission - * @property Finition $finition - * @property Model $model - * @property int $owner_id - * @property Adherent $owner - * @property Picture $picture - * @property History $history */ class Auto { public const string TABLE = 'cars'; public const string PK = 'id_car'; - private Plugins $plugins; - private Db $zdb; + public const int FUEL_PETROL = 1; + public const int FUEL_DIESEL = 2; + public const int FUEL_GAS = 3; + public const int FUEL_ELECTRICITY = 4; + public const int FUEL_BIO = 5; + public const int FUEL_HYBRID = 6; - /** @var array */ - private array $fields = [ - 'id_car' => 'integer', - 'car_name' => 'string', - 'car_registration' => 'string', - 'car_first_registration_date' => 'date', - 'car_first_circulation_date' => 'date', - 'car_mileage' => 'integer', - 'car_comment' => 'string', - 'car_creation_date' => 'date', - 'car_chassis_number' => 'string', - 'car_seats' => 'integer', - 'car_horsepower' => 'integer', - 'car_engine_size' => 'integer', - 'car_fuel' => 'integer', - Color::PK => 'integer', - Body::PK => 'integer', - State::PK => 'integer', - Transmission::PK => 'integer', - Finition::PK => 'integer', - Model::PK => 'integer', - Adherent::PK => 'integer' + /** + * Fields that can be posted, in the order they are checked + * + * @var array + */ + private const array POSTED_FIELDS = [ + 'registration', + 'name', + 'first_registration_date', + 'first_circulation_date', + 'mileage', + 'comment', + 'chassis_number', + 'seats', + 'horsepower', + 'engine_size', + 'fuel', + 'finition', + 'color', + 'model', + 'transmission', + 'body', + 'state', + 'owner_id' ]; + private Plugins $plugins; + private Db $zdb; + /** @var array */ private array $required = [ 'name' => 1, @@ -95,60 +79,35 @@ class Auto 'fuel' => 1 ]; - private int $id; - private string $registration; - private string $name; - private string $first_registration_date; - private string $first_circulation_date; - private ?int $mileage; - private ?string $comment; - private ?string $chassis_number; - private ?int $seats; - private ?int $horsepower; - private ?int $engine_size; - private string $creation_date; + private ?int $id = null; + private ?string $registration = null; + private ?string $name = null; + private ?string $first_registration_date = null; + private ?string $first_circulation_date = null; + private ?int $mileage = null; + private ?string $comment = null; + private ?string $chassis_number = null; + private ?int $seats = null; + private ?int $horsepower = null; + private ?int $engine_size = null; + private ?string $creation_date = null; private ?int $fuel = null; //External objects - private Picture $picture; + private ?Picture $picture = null; private Finition $finition; private Color $color; private Model $model; private Transmission $transmission; private Body $body; - private History $history; + private ?History $history = null; private State $state; - private int $owner_id; + private ?int $owner_id = null; private Adherent $owner; - public const int FUEL_PETROL = 1; - public const int FUEL_DIESEL = 2; - public const int FUEL_GAS = 3; - public const int FUEL_ELECTRICITY = 4; - public const int FUEL_BIO = 5; - public const int FUEL_HYBRID = 6; - /** @var array */ private array $propnames; //textual properties names - //do we have to fire a history entry? - private bool $fire_history = false; - - /** - * @var array internal properties (not updatable outside the object) - */ - private array $internals = [ - 'id', - 'creation_date', - 'history', - 'picture', - 'propnames', - 'internals', - 'fields', - 'fire_history', - 'plugins', - 'zdb' - ]; /** @var array */ private array $errors = []; @@ -185,14 +144,11 @@ public function __construct(Plugins $plugins, Db $zdb, ?ArrayObject $args = null $this->model = new Model($this->zdb); $this->color = new Color($this->zdb); $this->state = new State($this->zdb); - $this->owner = new Adherent($this->zdb); $this->owner->disableAllDeps()->enableDep('parent'); $this->transmission = new Transmission($this->zdb); $this->finition = new Finition($this->zdb); - $this->picture = new Picture($this->plugins); $this->body = new Body($this->zdb); - $this->history = new History($this->zdb); if ($args instanceof ArrayObject) { $this->loadFromRS($args); } @@ -251,16 +207,13 @@ private function loadFromRS(ArrayObject $r): void $this->creation_date = (string)$r['car_creation_date']; $this->fuel = $r['car_fuel'] !== null ? (int)$r['car_fuel'] : null; //External objects - $this->picture = new Picture($this->plugins, $this->id); $this->finition->load((int)$r[Finition::PK]); $this->color->load((int)$r[Color::PK]); $this->model->load((int)$r[Model::PK]); $this->transmission->load((int)$r[Transmission::PK]); $this->body->load((int)$r[Body::PK]); - $this->owner_id = (int)$r[Adherent::PK]; - $this->owner->load($this->owner_id); $this->state->load((int)$r[State::PK]); - $this->history->load($this->id); + $this->setOwner((int)$r[Adherent::PK]); } /** @@ -271,7 +224,7 @@ private function loadFromRS(ArrayObject $r): void public function listFuels(): array { //TODO: make this list configurable? - $f = [ + return [ self::FUEL_PETROL => _T("Petrol", "auto"), self::FUEL_DIESEL => _T("Diesel", "auto"), self::FUEL_GAS => _T("Gas", "auto"), @@ -279,7 +232,6 @@ public function listFuels(): array self::FUEL_ELECTRICITY => _T("Electricity", "auto"), self::FUEL_BIO => _T("Bio", "auto") ]; - return $f; } /** @@ -297,54 +249,7 @@ public function store(bool $new = false): bool } try { - $values = []; - - foreach ($this->fields as $k => $v) { - switch ($k) { - case self::PK: - break; - case Color::PK: - $values[$k] = $this->color->getId(); - break; - case Body::PK: - $values[$k] = $this->body->getId(); - break; - case State::PK: - $values[$k] = $this->state->getId(); - break; - case Transmission::PK: - $values[$k] = $this->transmission->getId(); - break; - case Finition::PK: - $values[$k] = $this->finition->getId(); - break; - case Model::PK: - $values[$k] = $this->model->getId(); - break; - case Adherent::PK: - $values[$k] = $this->owner->id; - break; - default: - $propName = substr($k, 4, strlen($k)); - switch ($v) { - case 'string': - case 'date': - $values[$k] = $this->$propName ?? null; - break; - case 'integer': - $values[$k] = ( - (!empty($this->$propName)) - ? $this->$propName - : new Expression('NULL') - ); - break; - default: - $values[$k] = $this->$propName; - break; - } - break; - } - } + $values = $this->getStorableValues(); if ($new === true) { $insert = $this->zdb->insert(AUTO_PREFIX . self::TABLE); @@ -362,9 +267,8 @@ public function store(bool $new = false): bool // logging $hist->add( _T("New car added", "auto"), - strtoupper($this->name) + strtoupper((string)$this->name) ); - $this->history->load((int)$this->id); } else { $hist->add(_T("Fail to add new car.", "auto")); throw new \Exception( @@ -384,37 +288,29 @@ public function store(bool $new = false): bool if ($edit->count() > 0) { $hist->add( _T("Car updated", "auto"), - strtoupper($this->name) + strtoupper((string)$this->name) ); } } //if all goes well, we check to add an entry into car's history - $h = $this->history->getLatest(); - if (!$new && $h !== false) { - foreach ($h as $k => $v) { - if ($k != 'history_date' && $this->$k != $v) { - //if one has been modified, we flag to add an entry event - $this->fire_history = true; + $history = $this->getHistory(); + $latest = $history->getLatest(); + $current = $this->getHistoryValues(); + $fire_history = $new; + if (!$new && $latest !== false) { + foreach ($current as $k => $v) { + if ($k !== 'history_date' && (string)$latest[$k] !== (string)$v) { + //if one has been modified, we add an entry + $fire_history = true; break; } } - } elseif ($new) { - //no history entry... yet! Let's create one. - $this->fire_history = true; } - if ($this->fire_history) { - $h_props = []; - foreach ($this->history->getFields() as $prop) { - if ($prop != 'history_date') { - $h_props[$prop] = $this->$prop; - } else { - $h_props[$prop] = date('Y-m-d H:i:s'); - } - } - $this->history->register($h_props); - $this->fire_history = false; + if ($fire_history) { + $history->register($current); + $history->load((int)$this->id); } return true; @@ -430,63 +326,50 @@ public function store(bool $new = false): bool } /** - * List object's properties + * Get values to store in database * - * @param bool $restrict true to exclude $this->internals from returned - * result, false otherwise. Default to false - * - * @return array List of properties + * @return array */ - private function getAllProperties(bool $restrict = false): array + public function getStorableValues(): array { - $result = []; - foreach (array_keys(get_class_vars(static::class)) as $key) { - if ( - !$restrict - || !in_array($key, $this->internals) - ) { - $result[] = $key; - } - } - return $result; + return [ + 'car_name' => $this->name, + 'car_registration' => $this->registration, + 'car_first_registration_date' => $this->first_registration_date, + 'car_first_circulation_date' => $this->first_circulation_date, + 'car_mileage' => $this->mileage, + 'car_comment' => $this->comment, + 'car_creation_date' => $this->creation_date, + 'car_chassis_number' => $this->chassis_number, + 'car_seats' => $this->seats, + 'car_horsepower' => $this->horsepower, + 'car_engine_size' => $this->engine_size, + 'car_fuel' => $this->fuel, + Color::PK => $this->color->getId(), + Body::PK => $this->body->getId(), + State::PK => $this->state->getId(), + Transmission::PK => $this->transmission->getId(), + Finition::PK => $this->finition->getId(), + Model::PK => $this->model->getId(), + Adherent::PK => $this->owner_id + ]; } /** - * Get object's properties. List only properties that can be modified - * externally (ie. not in $this->internals) + * Get values tracked in history, as they are now * - * @return array List of properties + * @return array */ - public function getProperties(): array + public function getHistoryValues(): array { - $properties = $this->getAllProperties(true); - $to_unset = ['required', 'errors']; - foreach ($to_unset as $prop) { - unset($properties[array_search($prop, $properties)]); - } - return $properties; - } - - /** - * Get year of first circulation - */ - public function getFirstCirculationYear(): ?int - { - if (empty($this->first_circulation_date)) { - return null; - } - return (int)substr($this->first_circulation_date, 0, 4); - } - - /** - * Get fuel label - */ - public function getFuelLabel(): ?string - { - if ($this->fuel === null) { - return null; - } - return $this->listFuels()[$this->fuel] ?? null; + return [ + self::PK => $this->id, + Adherent::PK => $this->owner_id, + 'history_date' => date('Y-m-d H:i:s'), + 'car_registration' => $this->registration, + Color::PK => $this->color->getId(), + State::PK => $this->state->getId() + ]; } /** @@ -494,7 +377,7 @@ public function getFuelLabel(): ?string */ public function hasPicture(): bool { - return $this->picture->hasPicture(); + return $this->getPicture()->hasPicture(); } /** @@ -504,8 +387,7 @@ public function hasPicture(): bool */ public function appropriateCar(Login $login): void { - $this->owner_id = $login->id; - $this->owner->load($this->owner_id); + $this->setOwner((int)$login->id); } /** @@ -524,128 +406,6 @@ public function getPropName(string $name): string } } - /** - * Global getter method - * - * @param string $name name of the property we want to retrieve - * - * @return mixed the called property - */ - public function __get(string $name): mixed - { - $forbidden = []; - if (!in_array($name, $forbidden)) { - switch ($name) { - case self::PK: - return $this->id; - case Adherent::PK: - return $this->owner->id; - case Color::PK: - return $this->color->getId(); - case State::PK: - return $this->state->getId(); - case 'car_registration': - return $this->registration; - case 'first_registration_date': - case 'first_circulation_date': - case 'creation_date': - if (isset($this->$name)) { - try { - $d = new \DateTime($this->$name); - return $d->format(_T("Y-m-d")); - } catch (\Exception $e) { - //oops, we've got a bad date :/ - Analog::log( - 'Bad date (' . $this->$name . ') | ' - . $e->getMessage(), - Analog::WARNING - ); - return $this->$name; - } - } - return null; - case 'picture': - return $this->picture; - default: - return $this->$name ?? ''; - } - } - - throw new \RuntimeException( - sprintf( - 'Unable to get property "%s::%s"!', - __CLASS__, - $name - ) - ); - } - - /** - * Global setter method - * - * @param string $name name of the property we want to assign a value to - * @param mixed $value a relevant value for the property - */ - public function __set(string $name, mixed $value): void - { - if (!in_array($name, $this->internals)) { - switch ($name) { - case 'finition': - $this->finition->load((int)$value); - break; - case 'color': - $this->color->load((int)$value); - break; - case 'model': - $this->model->load((int)$value); - break; - case 'transmission': - $this->transmission->load((int)$value); - break; - case 'body': - $this->body->load((int)$value); - break; - case 'owner_id': - $this->owner_id = (int)$value; - $this->owner->load($this->owner_id); - break; - case 'state': - $this->state->load((int)$value); - break; - default: - $this->$name = $value; - break; - } - } else { - Analog::log( - '[' . get_class($this) . '] Trying to set an internal property (`' - . $name . '`)', - Analog::INFO - ); - } - } - - /** - * Global isset method - * Required for twig to access properties via __get - * - * @param string $name name of the property we want to retrieve - */ - public function __isset(string $name): bool - { - $knowns = [ - self::PK, - Adherent::PK, - Color::PK, - State::PK - ]; - if (in_array($name, $knowns)) { - return true; - } - - return property_exists($this, $name); - } - /** * Check posted values validity * @@ -659,7 +419,7 @@ public function check(array $post, VehicleAccess $access): bool //check for required fields, and correct values $required = $this->getRequired(); - foreach ($this->getProperties() as $prop) { + foreach (self::POSTED_FIELDS as $prop) { $value = $post[$prop] ?? null; if (($value == '' || $value == null) && in_array($prop, array_keys($required))) { @@ -674,8 +434,8 @@ public function check(array $post, VehicleAccess $access): bool switch ($prop) { //string values with special check case 'registration': - if (mb_strlen($value) <= 10) { - $this->$prop = $value; + if (mb_strlen((string)$value) <= 10) { + $this->registration = (string)$value; } else { $this->errors[] = str_replace( [ @@ -686,7 +446,7 @@ public function check(array $post, VehicleAccess $access): bool [ '10', $this->getPropName($prop), - (string)mb_strlen($value) + (string)mb_strlen((string)$value) ], _T("- Maximum size for %field is %maxsize (current %cursize)!", "auto") ); @@ -694,30 +454,33 @@ public function check(array $post, VehicleAccess $access): bool break; //string values, no check case 'name': + $this->name = (string)$value; + break; case 'comment': + $this->comment = $value !== null && $value !== '' ? (string)$value : null; + break; case 'chassis_number': - $this->$prop = $value; + $this->chassis_number = $value !== null && $value !== '' ? (string)$value : null; break; //dates case 'first_registration_date': case 'first_circulation_date': - try { - $d = \DateTime::createFromFormat(__("Y-m-d"), $value); - if ($d === false) { - //try with non localized date - $d = \DateTime::createFromFormat("Y-m-d", $value); - if ($d === false) { - throw new \Exception('Incorrect format'); - } - } - $this->$prop = $d->format('Y-m-d'); - } catch (\Throwable $e) { + $d = \DateTime::createFromFormat(__("Y-m-d"), (string)$value); + if ($d === false) { + //try with non localized date + $d = \DateTime::createFromFormat("Y-m-d", (string)$value); + } + if ($d === false) { $this->errors[] = sprintf( //TRANS: %1$s is the date format, %2$s is the field name _T('- Wrong date format (%1$s) for %2$s!'), __("Y-m-d"), $this->getPropName($prop) ); + } elseif ($prop === 'first_registration_date') { + $this->first_registration_date = $d->format('Y-m-d'); + } else { + $this->first_circulation_date = $d->format('Y-m-d'); } break; //numeric values @@ -725,9 +488,12 @@ public function check(array $post, VehicleAccess $access): bool case 'seats': case 'horsepower': case 'engine_size': - if (is_numeric(str_replace(' ', '', $value ?? ''))) { - $this->$prop = (int)$value; - } elseif ($value != '') { + $number = str_replace(' ', '', (string)$value); + if ($number === '') { + $this->$prop = null; + } elseif (is_numeric($number)) { + $this->$prop = (int)$number; + } else { $this->errors[] = str_replace( '%s', '' . $this->getPropName($prop) . '', @@ -737,7 +503,7 @@ public function check(array $post, VehicleAccess $access): bool break; //constants case 'fuel': - if (in_array($value, array_keys($this->listFuels()))) { + if (in_array((int)$value, array_keys($this->listFuels()), true)) { $this->fuel = (int)$value; } else { $this->errors[] = _T("- You must choose a fuel in the list", "auto"); @@ -750,23 +516,16 @@ public function check(array $post, VehicleAccess $access): bool case 'transmission': case 'body': case 'state': - if ($value > 0) { - $this->$prop->load((int)$value); - } else { - $class = 'GaletteAuto\\' . ucwords($prop); - $name = $class::FIELD; + if ((int)$value <= 0 || !$this->$prop->load((int)$value)) { $this->errors[] = str_replace( '%s', - '' . $this->getPropName($name) . '', + '' . $this->getPropName($prop) . '', _T("- You must choose a %s in the list", "auto") ); } break; - case 'owner': - //owner is not a property that can be set. - break; case 'owner_id': - if (isset($post['change_owner']) || !isset($this->id)) { + if (isset($post['change_owner']) || $this->id === null) { $value = (int)$value; if (!$access->isManager()) { //simple members only own their vehicles @@ -782,25 +541,16 @@ public function check(array $post, VehicleAccess $access): bool ); $this->errors[] = _T("- you cannot attach this car to this member", "auto"); } else { - $this->owner_id = $value; - $this->owner->load($value); + $this->setOwner($value); } } break; - default: - /** TODO: what's the default? */ - Analog::log( - 'Trying to edit an Auto property that is not handled in the source code! (prop is: ' - . $prop . ')', - Analog::ERROR - ); - break; }//switch }//foreach //delete photo if (isset($post['del_photo'])) { - if (!$this->picture->delete()) { + if (!$this->getPicture()->delete()) { $this->errors[] = _T("An error occurred while trying to delete car's photo", "auto"); } @@ -850,4 +600,228 @@ public function handleFiles(array $files): bool return !count($this->errors); } + + /** + * Get vehicle ID + */ + public function getId(): ?int + { + return $this->id; + } + + /** + * Get vehicle name + */ + public function getName(): ?string + { + return $this->name; + } + + /** + * Get registration + */ + public function getRegistration(): ?string + { + return $this->registration; + } + + /** + * Get first registration date, as Y-m-d + */ + public function getFirstRegistrationDate(): ?string + { + return $this->first_registration_date; + } + + /** + * Get first circulation date, as Y-m-d + */ + public function getFirstCirculationDate(): ?string + { + return $this->first_circulation_date; + } + + /** + * Get year of first circulation + */ + public function getFirstCirculationYear(): ?int + { + if (empty($this->first_circulation_date)) { + return null; + } + return (int)substr($this->first_circulation_date, 0, 4); + } + + /** + * Get creation date, as Y-m-d + */ + public function getCreationDate(): ?string + { + return $this->creation_date; + } + + /** + * Get mileage + */ + public function getMileage(): ?int + { + return $this->mileage; + } + + /** + * Get comment + */ + public function getComment(): ?string + { + return $this->comment; + } + + /** + * Get chassis number + */ + public function getChassisNumber(): ?string + { + return $this->chassis_number; + } + + /** + * Get number of seats + */ + public function getSeats(): ?int + { + return $this->seats; + } + + /** + * Get horsepower + */ + public function getHorsepower(): ?int + { + return $this->horsepower; + } + + /** + * Get engine size + */ + public function getEngineSize(): ?int + { + return $this->engine_size; + } + + /** + * Get fuel, one of the FUEL_* constants + */ + public function getFuel(): ?int + { + return $this->fuel; + } + + /** + * Get fuel label + */ + public function getFuelLabel(): ?string + { + if ($this->fuel === null) { + return null; + } + return $this->listFuels()[$this->fuel] ?? null; + } + + /** + * Get model + */ + public function getModel(): Model + { + return $this->model; + } + + /** + * Get color + */ + public function getColor(): Color + { + return $this->color; + } + + /** + * Get state + */ + public function getState(): State + { + return $this->state; + } + + /** + * Get transmission + */ + public function getTransmission(): Transmission + { + return $this->transmission; + } + + /** + * Get finition + */ + public function getFinition(): Finition + { + return $this->finition; + } + + /** + * Get body + */ + public function getBody(): Body + { + return $this->body; + } + + /** + * Get owner ID + */ + public function getOwnerId(): ?int + { + return $this->owner_id; + } + + /** + * Get owner + */ + public function getOwner(): Adherent + { + return $this->owner; + } + + /** + * Set owner + * + * @param int $id_adh Member ID + */ + public function setOwner(int $id_adh): self + { + $this->owner_id = $id_adh; + $this->owner->load($id_adh); + return $this; + } + + /** + * Get picture + */ + public function getPicture(): Picture + { + if ($this->picture === null) { + $this->picture = new Picture($this->plugins, $this->id); + } + return $this->picture; + } + + /** + * Get history + */ + public function getHistory(): History + { + if ($this->history === null) { + $this->history = new History($this->zdb, $this->id); + } + return $this->history; + } } diff --git a/lib/GaletteAuto/Controllers/Controller.php b/lib/GaletteAuto/Controllers/Controller.php index 649f789..377d522 100644 --- a/lib/GaletteAuto/Controllers/Controller.php +++ b/lib/GaletteAuto/Controllers/Controller.php @@ -279,9 +279,9 @@ protected function listVehicles( $params['history_allowed'] = []; foreach ($params['autos'] as $vehicle) { if ($vehicle instanceof Auto) { - $params['public_owners'][$vehicle->id] = $access->isOwnerPublic($vehicle->owner); - $params['history_allowed'][$vehicle->id] = $this->login->isLogged() - && $access->canManageMember($vehicle->owner_id); + $params['public_owners'][$vehicle->getId()] = $access->isOwnerPublic($vehicle->getOwner()); + $params['history_allowed'][$vehicle->getId()] = $this->login->isLogged() + && $access->canManageMember($vehicle->getOwnerId()); } } } @@ -330,7 +330,7 @@ public function showAddEditVehicle(Request $request, Response $response, string $auto = new Auto($this->plugins, $this->zdb); if (!$is_new) { - if (!$auto->load((int)$id) || !$this->getAccess()->canManageMember($auto->owner_id)) { + if (!$auto->load((int)$id) || !$this->getAccess()->canManageMember($auto->getOwnerId())) { return $this->accessDenied($response, 'Trying to edit vehicle #' . $id); } } else { @@ -340,7 +340,7 @@ public function showAddEditVehicle(Request $request, Response $response, string && $this->getAccess()->isManager() && $this->getAccess()->canManageMember((int)$get['id_adh']) ) { - $auto->owner_id = (int)$get['id_adh']; + $auto->setOwner((int)$get['id_adh']); } else { $auto->appropriateCar($this->login); } @@ -353,7 +353,7 @@ public function showAddEditVehicle(Request $request, Response $response, string $title = ($is_new) ? _T("New vehicle", "auto") - : str_replace('%s', $auto->name, _T("Change vehicle '%s'", "auto")); + : str_replace('%s', $auto->getName(), _T("Change vehicle '%s'", "auto")); $mfilters = new ModelsList(); $models = new Models( @@ -369,13 +369,13 @@ public function showAddEditVehicle(Request $request, Response $response, string 'require_calendar' => true, 'require_dialog' => true, 'car' => $auto, - 'models' => $models->getList($auto->model->getBrand()->getId()), - 'brands' => $auto->model->getBrand()->getList(), - 'colors' => $auto->color->getList(), - 'bodies' => $auto->body->getList(), - 'transmissions' => $auto->transmission->getList(), - 'finitions' => $auto->finition->getList(), - 'states' => $auto->state->getList(), + 'models' => $models->getList($auto->getModel()->getBrand()->getId()), + 'brands' => $auto->getModel()->getBrand()->getList(), + 'colors' => $auto->getColor()->getList(), + 'bodies' => $auto->getBody()->getList(), + 'transmissions' => $auto->getTransmission()->getList(), + 'finitions' => $auto->getFinition()->getList(), + 'states' => $auto->getState()->getList(), 'fuels' => $auto->listFuels(), 'time' => time(), 'required' => $auto->getRequired() @@ -384,8 +384,8 @@ public function showAddEditVehicle(Request $request, Response $response, string // members $m = new Members(); $oid = null; - if ($auto->owner->id > 0) { - $oid = $auto->owner->id; + if ($auto->getOwnerId() > 0) { + $oid = $auto->getOwnerId(); } $members = $m->getDropdownMembers( $this->zdb, @@ -449,7 +449,7 @@ public function doAddEditVehicle(Request $request, Response $response, string $a $auto = new Auto($this->plugins, $this->zdb); if (!$is_new) { - if (!$auto->load((int)$id) || !$this->getAccess()->canManageMember($auto->owner_id)) { + if (!$auto->load((int)$id) || !$this->getAccess()->canManageMember($auto->getOwnerId())) { return $this->accessDenied($response, 'Trying to store vehicle #' . $id); } } @@ -466,7 +466,7 @@ public function doAddEditVehicle(Request $request, Response $response, string $a $error_detected[] = _T("- An error has occurred while saving vehicle in the database.", "auto"); } else { $success_detected[] = _T("Vehicle has been saved!", "auto"); - $route = $this->getListRoute($auto->owner_id); + $route = $this->getListRoute($auto->getOwnerId()); if (!$auto->handleFiles($request->getUploadedFiles())) { $warning_detected = $auto->getErrors(); } @@ -522,7 +522,7 @@ public function vehicleHistory(Request $request, Response $response, int $id): R { $history = new History($this->zdb, $id); $auto = new Auto($this->plugins, $this->zdb); - if (!$auto->load((int)$history->getCarId()) || !$this->getAccess()->canManageMember($auto->owner_id)) { + if (!$auto->load((int)$history->getCarId()) || !$this->getAccess()->canManageMember($auto->getOwnerId())) { return $this->accessDenied($response, 'Trying to show history of vehicle #' . $id); } @@ -572,10 +572,10 @@ public function ajaxModels(Request $request, Response $response): Response public function removeVehicle(Request $request, Response $response, int $id): Response { $auto = new Auto($this->plugins, $this->zdb); - if (!$auto->load($id) || !$this->getAccess()->canManageMember($auto->owner_id)) { + if (!$auto->load($id) || !$this->getAccess()->canManageMember($auto->getOwnerId())) { return $this->accessDenied($response, 'Trying to remove vehicle #' . $id); } - $route = $this->getListRoute($auto->owner_id); + $route = $this->getListRoute($auto->getOwnerId()); $data = [ 'id' => $id, @@ -591,9 +591,9 @@ public function removeVehicle(Request $request, Response $response, int $id): Re 'mode' => $this->isAjax($request) ? 'ajax' : '', 'page_title' => sprintf( _T('Remove vehicle %1$s', 'auto'), - $auto->name + $auto->getName() ), - 'form_url' => $this->routeparser->urlFor('doRemoveVehicle', ['id' => (string)$auto->id]), + 'form_url' => $this->routeparser->urlFor('doRemoveVehicle', ['id' => (string)$auto->getId()]), 'cancel_uri' => $route, 'data' => $data ] diff --git a/templates/default/public_vehicles_list.html.twig b/templates/default/public_vehicles_list.html.twig index 79db1b2..e2db664 100644 --- a/templates/default/public_vehicles_list.html.twig +++ b/templates/default/public_vehicles_list.html.twig @@ -37,7 +37,7 @@ {% block body %} {% for auto in autos %} - {% set brand = auto.model.brand %} + {% set brand = auto.getModel().getBrand() %} @@ -45,38 +45,38 @@ {% set photo_width = auto.picture.getOptimalWidth() %} {% set thumb_width = min(photo_width, 80) %} {{ auto.name }} - {{ auto.name }} - {% if history_allowed[auto.id] %} - {% endif %} {# members kept out of the members list are not named #} - {{ public_owners[auto.id] ? auto.owner.sfullname : '—' }} + {{ public_owners[auto.getId()] ? auto.getOwner().sfullname : '—' }} {{ brand.getValue() }} - {{ auto.model.model }} + {{ auto.getModel().getModel() }} {# what the car is, then how it is built #} - {% set identity = [auto.getFirstCirculationYear(), auto.body.getValue(), auto.finition.getValue()]|filter(v => v) %} + {% set identity = [auto.getFirstCirculationYear(), auto.getBody().getValue(), auto.getFinition().getValue()]|filter(v => v) %} {% set specs = [] %} - {% if auto.engine_size %} - {% set specs = specs|merge([_T("%1$s cc", "auto")|replace({'%1$s': auto.engine_size|format_number({}, 'decimal', 'default', i18n.getWebID())})]) %} + {% if auto.getEngineSize() %} + {% set specs = specs|merge([_T("%1$s cc", "auto")|replace({'%1$s': auto.getEngineSize()|format_number({}, 'decimal', 'default', i18n.getWebID())})]) %} {% endif %} - {% if auto.horsepower %} - {% set specs = specs|merge([_T("%1$s hp", "auto")|replace({'%1$s': auto.horsepower|format_number({}, 'decimal', 'default', i18n.getWebID())})]) %} + {% if auto.getHorsepower() %} + {% set specs = specs|merge([_T("%1$s hp", "auto")|replace({'%1$s': auto.getHorsepower()|format_number({}, 'decimal', 'default', i18n.getWebID())})]) %} {% endif %} - {% set specs = specs|merge([auto.getFuelLabel(), auto.transmission.getValue(), auto.color.getValue()]|filter(v => v)) %} + {% set specs = specs|merge([auto.getFuelLabel(), auto.getTransmission().getValue(), auto.getColor().getValue()]|filter(v => v)) %} {% if identity|length > 0 %} {{ identity|join(' · ') }} {% endif %} diff --git a/templates/default/vehicles.html.twig b/templates/default/vehicles.html.twig index 50f0af9..8ca5a9b 100644 --- a/templates/default/vehicles.html.twig +++ b/templates/default/vehicles.html.twig @@ -10,7 +10,7 @@ {% if mode == 'new' %} {% set action = url_for("doVehicleAdd") %} {% else %} - {% set action = url_for("doVehicleEdit", {"id": car.id}) %} + {% set action = url_for("doVehicleEdit", {"id": car.getId()}) %} {% endif %}
@@ -21,7 +21,7 @@
{% include "components/forms/text.html.twig" with { id: 'name', - value: car.name, + value: car.getName(), label: _T("Name", "auto"), required: required.name is defined } %} @@ -33,7 +33,7 @@ {% include "components/forms/select.html.twig" with { id: 'brand', - value: car.model.brand.getId(), + value: car.getModel().brand.getId(), values: brand_list_values, label: _T("Brand", "auto"), required: required.brand is defined @@ -46,7 +46,7 @@ {% include "components/forms/select.html.twig" with { id: 'model', - value: car.model.id, + value: car.getModel().getId(), values: model_list_values, label: _T("Model", "auto"), required: required.model is defined @@ -54,28 +54,28 @@ {% include "components/forms/date.html.twig" with { id: 'first_registration_date', - value: car.first_registration_date, + value: car.getFirstRegistrationDate() ? car.getFirstRegistrationDate()|date(_T("Y-m-d")) : '', label: _T("First registration date", "auto"), title: _T("First registration date", "auto") } %} {% include "components/forms/date.html.twig" with { id: 'first_circulation_date', - value: car.first_circulation_date, + value: car.getFirstCirculationDate() ? car.getFirstCirculationDate()|date(_T("Y-m-d")) : '', label: _T("First circulation date", "auto"), title: _T("First circulation date", "auto") } %} {% include "components/forms/number.html.twig" with { id: 'mileage', - value: car.mileage, + value: car.getMileage(), label: _T("Mileage", "auto"), required: required.mileage is defined } %} {% include "components/forms/number.html.twig" with { id: 'seats', - value: car.seats, + value: car.getSeats(), label: _T("Seats", "auto"), required: required.seats is defined } %} @@ -89,7 +89,7 @@ {{ _T("Car's photo", "auto") }}
- {{ _T(
+ {{ _T(
{% if car.hasPicture() %} {% include "components/forms/checkbox.html.twig" with { id: 'del_photo', @@ -115,14 +115,14 @@
- {% if car.id %} + {% if car.getId() %}

- {{ car.owner.sfullname }} + {{ car.getOwner().sfullname }} {% if login.isAdmin() or login.isStaff() or login.isGroupManager() %} {# Does car's history should be visible by the actual owner? #} - @@ -140,19 +140,19 @@ {% if login.isAdmin() or login.isStaff() or login.isGroupManager() %} {% set cclass = 'field' %} - {% if car.id %}{% set cclass = cclass ~ ' displaynone' %}{% endif %} + {% if car.getId() %}{% 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, + 'value': car.getOwnerId(), 'component_class': cclass } %}

{% else %} - - {{ members.list[car.owner.id] }} + + {{ members.list[car.getOwnerId()] }} {% endif %} {% set color_list_values = {(-1): _T("Choose a color", "auto")} %} @@ -162,7 +162,7 @@ {% include "components/forms/select.html.twig" with { id: 'color', - value: car.color.getId(), + value: car.getColor().getId(), values: color_list_values, label: _T("Color", "auto"), required: required.color is defined @@ -175,7 +175,7 @@ {% include "components/forms/select.html.twig" with { id: 'state', - value: car.state.getId(), + value: car.getState().getId(), values: state_list_values, label: _T("State", "auto"), required: required.state is defined @@ -183,7 +183,7 @@ {% include "components/forms/text.html.twig" with { id: 'registration', - value: car.registration, + value: car.getRegistration(), label: _T("Registration", "auto"), required: required.registration is defined } %} @@ -204,7 +204,7 @@ {% include "components/forms/select.html.twig" with { id: 'body', - value: car.body.getId(), + value: car.getBody().getId(), values: body_list_values, label: _T("Body", "auto"), required: required.body is defined @@ -217,7 +217,7 @@ {% include "components/forms/select.html.twig" with { id: 'transmission', - value: car.transmission.getId(), + value: car.getTransmission().getId(), values: transmission_list_values, label: _T("Transmission", "auto"), required: required.transmission is defined @@ -230,7 +230,7 @@ {% include "components/forms/select.html.twig" with { id: 'finition', - value: car.finition.getId(), + value: car.getFinition().getId(), values: finition_list_values, label: _T("Finition", "auto"), required: required.finition is defined @@ -238,21 +238,21 @@ {% include "components/forms/text.html.twig" with { id: 'chassis_number', - value: car.chassis_number, + value: car.getChassisNumber(), label: _T("Chassis number", "auto"), required: required.chassis_number is defined } %} {% include "components/forms/number.html.twig" with { id: 'horsepower', - value: car.horsepower, + value: car.getHorsepower(), label: _T("Horsepower", "auto"), required: required.horsepower is defined } %} {% include "components/forms/number.html.twig" with { id: 'engine_size', - value: car.engine_size, + value: car.getEngineSize(), label: _T("Engine size", "auto"), required: required.engine_size is defined } %} @@ -264,7 +264,7 @@ {% include "components/forms/select.html.twig" with { id: 'fuel', - value: car.fuel, + value: car.getFuel(), values: fuel_list_values, label: _T("Fuel", "auto"), required: required.fuel is defined @@ -280,7 +280,7 @@
{% include "components/forms/textarea.html.twig" with { id: 'comment', - value: car.comment, + value: car.getComment(), label: _T("Comment", "auto"), required: required.comment is defined } %} @@ -292,7 +292,7 @@ {{ _T("Save") }} - +
{% endblock %} diff --git a/templates/default/vehicles_list.html.twig b/templates/default/vehicles_list.html.twig index b6ad7b6..c02785c 100644 --- a/templates/default/vehicles_list.html.twig +++ b/templates/default/vehicles_list.html.twig @@ -52,35 +52,35 @@ {% block body %} {% for auto in autos %} - {% set brand = auto.model.brand %} - {% set edit_link = url_for("vehicleEdit", {"id": auto.id}) %} + {% set brand = auto.getModel().getBrand() %} + {% set edit_link = url_for("vehicleEdit", {"id": auto.getId()}) %} - + - {{ auto.name }} - {{ auto.owner.sfullname }} + {{ auto.getName() }} + {{ auto.getOwner().sfullname }} {{ brand.getValue() }} - {{ auto.model.model }} + {{ auto.getModel().getModel() }} - {% if login.isAdmin() or login.isStaff() or auto.owner.id == login.id or login.isGroupManager() and preferences.pref_bool_groupsmanagers_edit_member %} + {% if login.isAdmin() or login.isStaff() or auto.getOwnerId() == login.id or login.isGroupManager() and preferences.pref_bool_groupsmanagers_edit_member %} {% set actions = [ { - 'label': _T("Edit %vehicle", "auto")|replace({"%vehicle": auto.name}), + 'label': _T("Edit %vehicle", "auto")|replace({"%vehicle": auto.getName()}), 'route': { 'name': 'vehicleEdit', - 'args': {'id': auto.id} + 'args': {'id': auto.getId()} }, 'icon': 'edit' }, { - 'label': _T("%vehiclename: remove from database", "auto")|replace({"%vehiclename": auto.name}), + 'label': _T("%vehiclename: remove from database", "auto")|replace({"%vehiclename": auto.getName()}), 'route': { 'name': 'removeVehicle', - 'args': {'id': auto.id}, + 'args': {'id': auto.getId()}, }, 'icon': 'red trash', 'extra_class': 'delete' diff --git a/tests/GaletteAuto/tests/units/Auto.php b/tests/GaletteAuto/tests/units/Auto.php index dfa302a..b553c4b 100644 --- a/tests/GaletteAuto/tests/units/Auto.php +++ b/tests/GaletteAuto/tests/units/Auto.php @@ -145,11 +145,11 @@ public function testCrud(): void $stored = $auto->store(true); $this->assertEquals([], $auto->getErrors()); $this->assertTrue($stored); - $auto_id = $auto->id; + $auto_id = $auto->getId(); //check history $history = new \GaletteAuto\History($this->zdb); - $this->assertTrue($history->load($auto->id)); + $this->assertTrue($history->load($auto->getId())); $this->assertCount(1, $history->getEntries()); $entry = $history->getEntries()[0]; @@ -169,7 +169,7 @@ public function testCrud(): void array_keys($entry) ); - $this->assertSame($auto->id, (int)$entry['id_car']); + $this->assertSame($auto->getId(), (int)$entry['id_car']); $this->assertSame($adh->id, (int)$entry['id_adh']); $this->assertSame('GA-123-TE', $entry['car_registration']); $this->assertSame('Grey', $entry['color']); @@ -210,11 +210,11 @@ public function testCrud(): void //check history $history = new \GaletteAuto\History($this->zdb); - $this->assertTrue($history->load($auto->id)); + $this->assertTrue($history->load($auto->getId())); $this->assertCount(2, $history->getEntries()); $entry = $history->getEntries()[1]; - $this->assertSame($auto->id, (int)$entry['id_car']); + $this->assertSame($auto->getId(), (int)$entry['id_car']); $this->assertSame($adh2->id, (int)$entry['id_adh']); $this->assertSame('GA-123-TE', $entry['car_registration']); $this->assertSame('Yellow', $entry['color']); @@ -243,7 +243,7 @@ public function testCrud(): void $stored = $auto->store(true); $this->assertEquals([], $auto->getErrors()); $this->assertTrue($stored); - $auto2_id = $auto->id; + $auto2_id = $auto->getId(); $this->assertTrue($history->load($auto2_id)); $this->assertCount(1, $history->getEntries()); @@ -253,7 +253,7 @@ public function testCrud(): void //sorted by name $this->assertSame( ['My car', 'Titine'], - array_map(fn($car) => $car->name, $autos->getList(true)) + array_map(fn($car) => $car->getName(), $autos->getList(true)) ); $this->assertTrue($autos->removeVehicles([$auto_id])); From ed3e904669d2071bbc95bd376eed25c810f1494c Mon Sep 17 00:00:00 2001 From: Johan Cwiklinski Date: Sat, 26 Sep 2026 14:50:32 +0200 Subject: [PATCH 5/6] Vehicles repository: joined list, owners in one query, store and remove in a transaction, no globals --- lib/GaletteAuto/Auto.php | 161 ++++----- lib/GaletteAuto/Autos.php | 284 ---------------- lib/GaletteAuto/Controllers/Controller.php | 51 +-- lib/GaletteAuto/Repository/Vehicles.php | 377 +++++++++++++++++++++ tests/GaletteAuto/tests/units/Auto.php | 24 +- 5 files changed, 475 insertions(+), 422 deletions(-) delete mode 100644 lib/GaletteAuto/Autos.php create mode 100644 lib/GaletteAuto/Repository/Vehicles.php diff --git a/lib/GaletteAuto/Auto.php b/lib/GaletteAuto/Auto.php index 435346c..e732ed3 100644 --- a/lib/GaletteAuto/Auto.php +++ b/lib/GaletteAuto/Auto.php @@ -103,7 +103,7 @@ class Auto private ?History $history = null; private State $state; private ?int $owner_id = null; - private Adherent $owner; + private ?Adherent $owner = null; /** @var array */ private array $propnames; //textual properties names @@ -144,8 +144,6 @@ public function __construct(Plugins $plugins, Db $zdb, ?ArrayObject $args = null $this->model = new Model($this->zdb); $this->color = new Color($this->zdb); $this->state = new State($this->zdb); - $this->owner = new Adherent($this->zdb); - $this->owner->disableAllDeps()->enableDep('parent'); $this->transmission = new Transmission($this->zdb); $this->finition = new Finition($this->zdb); $this->body = new Body($this->zdb); @@ -206,13 +204,18 @@ private function loadFromRS(ArrayObject $r): void $this->engine_size = $r['car_engine_size'] !== null ? (int)$r['car_engine_size'] : null; $this->creation_date = (string)$r['car_creation_date']; $this->fuel = $r['car_fuel'] !== null ? (int)$r['car_fuel'] : null; - //External objects - $this->finition->load((int)$r[Finition::PK]); - $this->color->load((int)$r[Color::PK]); - $this->model->load((int)$r[Model::PK]); - $this->transmission->load((int)$r[Transmission::PK]); - $this->body->load((int)$r[Body::PK]); - $this->state->load((int)$r[State::PK]); + //External objects, from the row when they have been joined + foreach (['finition', 'color', 'transmission', 'body', 'state'] as $property) { + $class = $this->$property::class; + if (isset($r[$class::FIELD])) { + $this->$property->loadFromRow($r); + } else { + $this->$property->load((int)$r[$class::PK]); + } + } + $this->model = isset($r[Model::FIELD]) + ? new Model($this->zdb, $r) + : new Model($this->zdb, (int)$r[Model::PK]); $this->setOwner((int)$r[Adherent::PK]); } @@ -234,97 +237,6 @@ public function listFuels(): array ]; } - /** - * Stores the vehicle in the database - * - * @param bool $new true if it's a new record, false to update on - * that already exists. Defaults to false - */ - public function store(bool $new = false): bool - { - global $hist; - - if ($new) { - $this->creation_date = date('Y-m-d'); - } - - try { - $values = $this->getStorableValues(); - - if ($new === true) { - $insert = $this->zdb->insert(AUTO_PREFIX . self::TABLE); - $insert->values($values); - $add = $this->zdb->execute($insert); - - if ($add->count() > 0) { - /** @phpstan-ignore-next-line */ - $this->id = (int)$this->zdb->driver->getLastGeneratedValue( - $this->zdb->isPostgres() - ? PREFIX_DB . AUTO_PREFIX . self::TABLE . '_id_seq' - : null - ); - - // logging - $hist->add( - _T("New car added", "auto"), - strtoupper((string)$this->name) - ); - } else { - $hist->add(_T("Fail to add new car.", "auto")); - throw new \Exception( - 'An error occurred inserting new car!' - ); - } - } else { - $update = $this->zdb->update(AUTO_PREFIX . self::TABLE); - $update->set($values)->where( - [ - self::PK => $this->id - ] - ); - $edit = $this->zdb->execute($update); - //edit == 0 does not mean there were an error, but that there - //were nothing to change - if ($edit->count() > 0) { - $hist->add( - _T("Car updated", "auto"), - strtoupper((string)$this->name) - ); - } - } - - //if all goes well, we check to add an entry into car's history - $history = $this->getHistory(); - $latest = $history->getLatest(); - $current = $this->getHistoryValues(); - $fire_history = $new; - if (!$new && $latest !== false) { - foreach ($current as $k => $v) { - if ($k !== 'history_date' && (string)$latest[$k] !== (string)$v) { - //if one has been modified, we add an entry - $fire_history = true; - break; - } - } - } - - if ($fire_history) { - $history->register($current); - $history->load((int)$this->id); - } - - return true; - } catch (\Exception $e) { - Analog::log( - '[' . get_class($this) . '] An error has occurred ' - . (($new) ? 'inserting' : 'updating') . ' car | ' - . $e->getMessage(), - Analog::ERROR - ); - return false; - } - } - /** * Get values to store in database * @@ -784,10 +696,17 @@ public function getOwnerId(): ?int } /** - * Get owner + * Get owner, loaded on first call */ public function getOwner(): Adherent { + if ($this->owner === null) { + $this->owner = new Adherent($this->zdb); + $this->owner->disableAllDeps(); + if ($this->owner_id !== null && $this->owner_id > 0) { + $this->owner->load($this->owner_id); + } + } return $this->owner; } @@ -799,7 +718,43 @@ public function getOwner(): Adherent public function setOwner(int $id_adh): self { $this->owner_id = $id_adh; - $this->owner->load($id_adh); + $this->owner = null; + return $this; + } + + /** + * Set owner from an already loaded member + * + * @param Adherent $owner Owner + */ + public function setOwnerMember(Adherent $owner): self + { + $this->owner_id = (int)$owner->id; + $this->owner = $owner; + return $this; + } + + /** + * Set ID, once stored + * + * @param int $id Vehicle ID + */ + public function setId(int $id): self + { + $this->id = $id; + $this->history = null; + $this->picture = null; + return $this; + } + + /** + * Set creation date + * + * @param string $date Date, as Y-m-d + */ + public function setCreationDate(string $date): self + { + $this->creation_date = $date; return $this; } diff --git a/lib/GaletteAuto/Autos.php b/lib/GaletteAuto/Autos.php deleted file mode 100644 index 841cfb2..0000000 --- a/lib/GaletteAuto/Autos.php +++ /dev/null @@ -1,284 +0,0 @@ - - */ -class Autos -{ - public const string TABLE = Auto::TABLE; - public const string PK = Auto::PK; - - private Plugins $plugins; - private Db $zdb; - private ?int $count = null; - - /** - * Constructor - * - * @param Plugins $plugins Plugins instance - * @param Db $zdb Database instance - */ - public function __construct(Plugins $plugins, Db $zdb) - { - $this->plugins = $plugins; - $this->zdb = $zdb; - } - - /** - * Remove specified vehicles - * - * @param int|array $ids Vehicles identifiers to delete - */ - public function removeVehicles(int|array $ids): bool - { - global $hist; - - $list = is_array($ids) ? $ids : [$ids]; - - try { - $this->zdb->connection->beginTransaction(); - - //Retrieve some information - $select = $this->zdb->select(AUTO_PREFIX . self::TABLE, 'a'); - $select->columns( - [ - self::PK, - 'car_name' - ] - )->join( - ['b' => PREFIX_DB . AUTO_PREFIX . Model::TABLE], - 'a.' . Model::PK . ' = b.' . Model::PK, - ['model'] - )->join( - ['c' => PREFIX_DB . AUTO_PREFIX . Brand::TABLE], - 'b.' . Brand::PK . ' = c.' . Brand::PK, - ['brand'] - )->where->in(self::PK, $list); - - $vehicles = $this->zdb->execute($select); - - $infos = null; - foreach ($vehicles as $vehicle) { - $str_v = $vehicle->id_car . ' - ' . $vehicle->car_name - . ' (' . $vehicle->brand . ' ' . $vehicle->model . ')'; - $infos .= $str_v . "\n"; - - $p = new Picture($this->plugins, $vehicle->id_car); - if ($p->hasPicture()) { - if (!$p->delete()) { - Analog::log( - 'Unable to delete picture for vehicle ' - . $str_v, - Analog::ERROR - ); - throw new \Exception( - 'Unable to delete picture for vehicle ' - . $str_v - ); - } else { - $hist->add( - _T("Vehicle picture deleted", "auto"), - $str_v - ); - } - } - } - - //delete vehicles history - $delete = $this->zdb->delete(AUTO_PREFIX . History::TABLE); - $delete->where->in(self::PK, $list); - $this->zdb->execute($delete); - - //delete vehicles - $delete = $this->zdb->delete(AUTO_PREFIX . self::TABLE); - $delete->where->in(self::PK, $list); - $this->zdb->execute($delete); - - //add a history entry - $hist->add( - _T("Delete vehicles cards", "auto"), - $infos - ); - - //commit all changes - $this->zdb->connection->commit(); - return true; - } catch (\Exception $e) { - $this->zdb->connection->rollBack(); - Analog::log( - 'Unable to delete selected vehicle(s) |' - . $e->getMessage(), - Analog::ERROR - ); - return false; - } - } - - /** - * Get vehicles list for specified member - * - * @param int $id_adh Members id - * @param ?AutosList $filters Filters - * - * @return array Vehicles list - */ - public function getMemberList(int $id_adh, ?AutosList $filters): array - { - return $this->getList(true, false, $filters, $id_adh); - } - - /** - * Get the list of all vehicles - * - * @param bool $as_autos return the results as an array of Auto object - * @param bool $mine show only current logged member cars - * @param ?AutosList $filters Filters - * @param ?int $id_adh Member id - * @param bool $public Get public list - * - * @return array|ResultSet - */ - public function getList( - bool $as_autos = false, - bool $mine = false, - ?AutosList $filters = null, - ?int $id_adh = null, - bool $public = false - ): array|ResultSet { - global $login; - - try { - $select = $this->zdb->select(AUTO_PREFIX . self::TABLE, 'a'); - - //restrict on user self vehicles when not admin, or if admin and requested 'my vehicles' - //the public list has every vehicle: who sees it is up to the - //visibility of the page, and owners are only named if they are public - $on_logged = false; - if ($mine) { - $on_logged = true; - } elseif (!$public && !$login->isAdmin() && !$login->isStaff() && $login->isGroupManager()) { - $groups = new \Galette\Repository\Groups($this->zdb, $login); - $managed_users = $groups->getManagerUsers(); - if (count($managed_users)) { - $managed_users[] = $login->id; - $select->where->in(Adherent::PK, $managed_users); - } else { - $on_logged = true; - } - } elseif (!$public && !$login->isAdmin() && !$login->isStaff()) { - $on_logged = true; - } - - if ($on_logged) { - $select->where( - [ - Adherent::PK => $login->id - ] - ); - } - - //restrict on specified user vehicles if an id has been provided - if ($id_adh !== null) { - $select->where( - [ - Adherent::PK => $id_adh - ] - ); - } - - $this->proceedCount($select, $filters); - $select->order(['a.car_name ASC', 'a.' . self::PK . ' ASC']); - - if ($filters !== null) { - $filters->setLimits($select); - } - - $results = $this->zdb->execute($select); - $autos = []; - if ($as_autos) { - foreach ($results as $row) { - $autos[] = new Auto($this->plugins, $this->zdb, $row); - } - } else { - $autos = $results; - } - return $autos; - } catch (\Exception $e) { - Analog::log( - '[' . get_class($this) . '] Cannot list Autos | ' - . $e->getMessage(), - Analog::ERROR - ); - throw $e; - } - } - - /** - * Count vehicles from the query - * - * @param Select $select Original select - * @param ?AutosList $filters Filters - */ - private function proceedCount(Select $select, ?AutosList $filters): void - { - try { - $countSelect = clone $select; - $countSelect->reset($countSelect::COLUMNS); - $countSelect->reset($countSelect::ORDER); - $countSelect->reset($countSelect::HAVING); - $countSelect->columns( - [ - 'count' => new Expression('count(DISTINCT a.' . self::PK . ')') - ] - ); - - $have = $select->having; - if ($have->count() > 0) { - foreach ($have->getPredicates() as $h) { - $countSelect->where($h); - } - } - - $results = $this->zdb->execute($countSelect); - $this->count = (int)$results->current()->count; - if ($this->count > 0 && $filters !== null) { - $filters->setCounter($this->count); - } - } catch (\Exception $e) { - Analog::log( - 'Cannot count vehicles | ' . $e->getMessage(), - Analog::WARNING - ); - throw $e; - } - } - - /** - * Get count for list - */ - public function getCount(): int - { - return $this->count; - } -} diff --git a/lib/GaletteAuto/Controllers/Controller.php b/lib/GaletteAuto/Controllers/Controller.php index 377d522..6388abc 100644 --- a/lib/GaletteAuto/Controllers/Controller.php +++ b/lib/GaletteAuto/Controllers/Controller.php @@ -13,7 +13,6 @@ use Analog\Analog; use Galette\Repository\Members; use GaletteAuto\Auto; -use GaletteAuto\Autos; use GaletteAuto\History; use GaletteAuto\Model; use GaletteAuto\Picture; @@ -26,6 +25,7 @@ use GaletteAuto\Filters\ModelsList; use GaletteAuto\Filters\AutosList; use GaletteAuto\Repository\Models; +use GaletteAuto\Repository\Vehicles; use DI\Attribute\Inject; /** @@ -67,6 +67,14 @@ protected function accessDenied(Response $response, string $log): Response ); } + /** + * Get vehicles repository + */ + protected function getVehicles(): Vehicles + { + return new Vehicles($this->plugins, $this->zdb, $this->login, $this->history); + } + /** * Can current user manage all the vehicles? * @@ -78,12 +86,7 @@ protected function canManageVehicles(array $ids): bool return false; } - $select = $this->zdb->select(AUTO_PREFIX . Auto::TABLE); - $select->columns([Auto::PK, Adherent::PK])->where->in(Auto::PK, $ids); - $owners = []; - foreach ($this->zdb->execute($select) as $row) { - $owners[(int)$row[Auto::PK]] = (int)$row[Adherent::PK]; - } + $owners = $this->getVehicles()->getOwners($ids); if (count($owners) !== count(array_unique($ids))) { //some vehicles do not exist @@ -228,7 +231,7 @@ protected function listVehicles( } } - $auto = new Autos($this->plugins, $this->zdb); + $vehicles = $this->getVehicles(); //the public page paginates on its own: a manager going there must not //land on the page, or the number of rows, of the management list $session_key = $public ? 'public_vehicles_filters' : 'vehicles_filters'; @@ -264,13 +267,11 @@ protected function listVehicles( 'require_dialog' => true ]; - if ($id_adh === null) { - $params['autos'] = $auto->getList(true, $mine, $afilters, null, $public); - } else { + if ($id_adh !== null) { $params['id_adh'] = $id_adh; - $params['autos'] = $auto->getMemberList($id_adh, $afilters); } - $params['count_vehicles'] = $auto->getCount(); + $params['autos'] = $vehicles->getList($afilters, $id_adh, $mine, $public); + $params['count_vehicles'] = $vehicles->getCount(); if ($public) { $access = $this->getAccess(); @@ -278,11 +279,9 @@ protected function listVehicles( //history is shown to whoever may see it from the vehicle form $params['history_allowed'] = []; foreach ($params['autos'] as $vehicle) { - if ($vehicle instanceof Auto) { - $params['public_owners'][$vehicle->getId()] = $access->isOwnerPublic($vehicle->getOwner()); - $params['history_allowed'][$vehicle->getId()] = $this->login->isLogged() - && $access->canManageMember($vehicle->getOwnerId()); - } + $params['public_owners'][$vehicle->getId()] = $access->isOwnerPublic($vehicle->getOwner()); + $params['history_allowed'][$vehicle->getId()] = $this->login->isLogged() + && $access->canManageMember((int)$vehicle->getOwnerId()); } } @@ -462,7 +461,13 @@ public function doAddEditVehicle(Request $request, Response $response, string $a $route = $this->routeparser->urlFor('vehiclesList'); //if no errors were thrown, we can store the car if (count($error_detected) == 0) { - if (!$auto->store($is_new)) { + try { + $this->getVehicles()->store($auto); + $stored = true; + } catch (\Throwable $e) { + $stored = false; + } + if (!$stored) { $error_detected[] = _T("- An error has occurred while saving vehicle in the database.", "auto"); } else { $success_detected[] = _T("Vehicle has been saved!", "auto"); @@ -701,8 +706,12 @@ public function doRemoveVehicle(Request $request, Response $response): Response return $this->accessDenied($response, 'Trying to remove vehicles #' . implode(', #', $ids)); } - $autos = new Autos($this->plugins, $this->zdb); - $del = $autos->removeVehicles($ids); + try { + $this->getVehicles()->remove($ids); + $del = true; + } catch (\Throwable $e) { + $del = false; + } unset($this->session->filter_vehicles); if ($del !== true) { diff --git a/lib/GaletteAuto/Repository/Vehicles.php b/lib/GaletteAuto/Repository/Vehicles.php new file mode 100644 index 0000000..65b17f4 --- /dev/null +++ b/lib/GaletteAuto/Repository/Vehicles.php @@ -0,0 +1,377 @@ + + */ +class Vehicles +{ + private int $count = 0; + + /** + * Constructor + * + * @param Plugins $plugins Plugins instance + * @param Db $zdb Database instance + * @param Login $login Login instance + * @param CoreHistory $history Galette history, logs changes + */ + public function __construct( + private readonly Plugins $plugins, + private readonly Db $zdb, + private readonly Login $login, + private readonly CoreHistory $history + ) { + } + + /** + * Get vehicles, with their properties and owner, in a few queries + * + * Without member nor public restriction, vehicles are the ones current + * user manages: every one for staff, the ones of the members of their + * groups for group managers, their own ones for others. + * + * @param ?AutosList $filters Filters + * @param ?int $id_adh Only vehicles of this member + * @param bool $mine Only current user vehicles + * @param bool $public Public list: every vehicle + * + * @return array + */ + public function getList( + ?AutosList $filters = null, + ?int $id_adh = null, + bool $mine = false, + bool $public = false + ): array { + $select = $this->buildSelect(); + + if ($mine) { + $select->where(['a.' . Adherent::PK => $this->login->id]); + } elseif ($id_adh !== null) { + $select->where(['a.' . Adherent::PK => $id_adh]); + } elseif (!$public && !$this->login->isAdmin() && !$this->login->isStaff()) { + $members = [$this->login->id]; + if ($this->login->isGroupManager()) { + $groups = new Groups($this->zdb, $this->login); + $members = array_merge($members, $groups->getManagerUsers() ?: []); + } + $select->where->in('a.' . Adherent::PK, $members); + } + + $this->proceedCount($select, $filters); + $select->order(['a.car_name ASC', 'a.' . Auto::PK . ' ASC']); + $filters?->setLimits($select); + + $rows = $this->zdb->execute($select)->toArray(); + $owners = $this->loadOwners(array_column($rows, Adherent::PK)); + + $vehicles = []; + foreach ($rows as $row) { + $vehicle = new Auto($this->plugins, $this->zdb, new ArrayObject($row)); + $id_owner = (int)$row[Adherent::PK]; + if (isset($owners[$id_owner])) { + $vehicle->setOwnerMember($owners[$id_owner]); + } + $vehicles[] = $vehicle; + } + return $vehicles; + } + + /** + * Get count for last list + */ + public function getCount(): int + { + return $this->count; + } + + /** + * Get vehicles owners + * + * @param array $ids Vehicles IDs + * + * @return array Owners IDs, per vehicle ID; missing vehicles are left out + */ + public function getOwners(array $ids): array + { + if (count($ids) === 0) { + return []; + } + + $select = $this->zdb->select(AUTO_PREFIX . Auto::TABLE); + $select->columns([Auto::PK, Adherent::PK])->where->in(Auto::PK, $ids); + $owners = []; + foreach ($this->zdb->execute($select) as $row) { + $owners[(int)$row[Auto::PK]] = (int)$row[Adherent::PK]; + } + return $owners; + } + + /** + * Store a vehicle, and its history when a tracked value changed + * + * @param Auto $vehicle Vehicle + * + * @throws \Throwable + */ + public function store(Auto $vehicle): void + { + $new = $vehicle->getId() === null; + $transaction = !$this->zdb->inTransaction(); + + try { + if ($transaction) { + $this->zdb->beginTransaction(); + } + + if ($new) { + $vehicle->setCreationDate(date('Y-m-d')); + $insert = $this->zdb->insert(AUTO_PREFIX . Auto::TABLE); + $insert->values($vehicle->getStorableValues()); + $this->zdb->execute($insert); + /** @phpstan-ignore-next-line */ + $vehicle->setId((int)$this->zdb->driver->getLastGeneratedValue( + $this->zdb->isPostgres() + ? PREFIX_DB . AUTO_PREFIX . Auto::TABLE . '_id_seq' + : null + )); + $this->history->add( + _T("New car added", "auto"), + strtoupper((string)$vehicle->getName()) + ); + } else { + $update = $this->zdb->update(AUTO_PREFIX . Auto::TABLE); + $update->set($vehicle->getStorableValues())->where([Auto::PK => $vehicle->getId()]); + //no affected rows does not mean an error, but nothing to change + if ($this->zdb->execute($update)->count() > 0) { + $this->history->add( + _T("Car updated", "auto"), + strtoupper((string)$vehicle->getName()) + ); + } + } + + $history = $vehicle->getHistory(); + $current = $vehicle->getHistoryValues(); + $latest = $new ? false : $history->getLatest(); + $changed = $new; + if ($latest !== false) { + foreach ($current as $key => $value) { + if ($key !== 'history_date' && (string)$latest[$key] !== (string)$value) { + $changed = true; + break; + } + } + } + if ($changed) { + $history->register($current); + $history->load((int)$vehicle->getId()); + } + + if ($transaction) { + $this->zdb->commit(); + } + } catch (\Throwable $e) { + if ($transaction) { + $this->zdb->rollback(); + } + Analog::log( + 'Unable to ' . ($new ? 'add' : 'update') . ' vehicle #' . ($vehicle->getId() ?? '') + . ' | ' . $e->getMessage(), + Analog::ERROR + ); + throw $e; + } + } + + /** + * Remove vehicles, with their history and photos + * + * @param array $ids Vehicles IDs + * + * @throws \Throwable + */ + public function remove(array $ids): void + { + $transaction = !$this->zdb->inTransaction(); + + try { + if ($transaction) { + $this->zdb->beginTransaction(); + } + + $select = $this->zdb->select(AUTO_PREFIX . Auto::TABLE, 'a'); + $select->columns([Auto::PK, 'car_name'])->join( + ['m' => PREFIX_DB . AUTO_PREFIX . Model::TABLE], + 'a.' . Model::PK . ' = m.' . Model::PK, + [Model::FIELD] + )->join( + ['b' => PREFIX_DB . AUTO_PREFIX . Brand::TABLE], + 'm.' . Brand::PK . ' = b.' . Brand::PK, + [Brand::FIELD] + )->where->in('a.' . Auto::PK, $ids); + + $infos = ''; + foreach ($this->zdb->execute($select) as $vehicle) { + $str_v = $vehicle[Auto::PK] . ' - ' . $vehicle['car_name'] + . ' (' . $vehicle[Brand::FIELD] . ' ' . $vehicle[Model::FIELD] . ')'; + $infos .= $str_v . "\n"; + + $picture = new Picture($this->plugins, (int)$vehicle[Auto::PK]); + if ($picture->hasPicture()) { + //file may be gone if the transaction is rolled back: it + //will be written again from the database when displayed + if (!$picture->delete(false)) { + throw new \RuntimeException('Unable to delete picture for vehicle ' . $str_v); + } + $this->history->add( + _T("Vehicle picture deleted", "auto"), + $str_v + ); + } + } + + $delete = $this->zdb->delete(AUTO_PREFIX . History::TABLE); + $delete->where->in(Auto::PK, $ids); + $this->zdb->execute($delete); + + $delete = $this->zdb->delete(AUTO_PREFIX . Auto::TABLE); + $delete->where->in(Auto::PK, $ids); + $this->zdb->execute($delete); + + $this->history->add( + _T("Delete vehicles cards", "auto"), + $infos + ); + + if ($transaction) { + $this->zdb->commit(); + } + } catch (\Throwable $e) { + if ($transaction) { + $this->zdb->rollback(); + } + Analog::log( + 'Unable to remove vehicles #' . implode(', #', $ids) . ' | ' . $e->getMessage(), + Analog::ERROR + ); + throw $e; + } + } + + /** + * Build vehicles select, joining their properties + */ + private function buildSelect(): Select + { + $select = $this->zdb->select(AUTO_PREFIX . Auto::TABLE, 'a'); + $joins = [ + 'co' => [Color::TABLE, Color::PK, [Color::FIELD]], + 'bo' => [Body::TABLE, Body::PK, [Body::FIELD]], + 'st' => [State::TABLE, State::PK, [State::FIELD]], + 'tr' => [Transmission::TABLE, Transmission::PK, [Transmission::FIELD]], + 'fi' => [Finition::TABLE, Finition::PK, [Finition::FIELD]], + 'mo' => [Model::TABLE, Model::PK, [Model::FIELD, Brand::PK]], + ]; + foreach ($joins as $alias => [$table, $pk, $columns]) { + $select->join( + [$alias => PREFIX_DB . AUTO_PREFIX . $table], + 'a.' . $pk . ' = ' . $alias . '.' . $pk, + $columns + ); + } + $select->join( + ['br' => PREFIX_DB . AUTO_PREFIX . Brand::TABLE], + 'mo.' . Brand::PK . ' = br.' . Brand::PK, + [Brand::FIELD] + ); + return $select; + } + + /** + * Load owners, in one query + * + * @param array $ids Members IDs + * + * @return array + */ + private function loadOwners(array $ids): array + { + $ids = array_unique(array_map('intval', $ids)); + if (count($ids) === 0) { + return []; + } + + $select = $this->zdb->select(Adherent::TABLE, 'a'); + $select->join( + ['s' => PREFIX_DB . Status::TABLE], + 'a.' . Status::PK . ' = s.' . Status::PK, + ['priorite_statut'] + )->where->in('a.' . Adherent::PK, $ids); + + $owners = []; + foreach ($this->zdb->execute($select) as $row) { + $owners[(int)$row[Adherent::PK]] = new Adherent($this->zdb, $row, false); + } + return $owners; + } + + /** + * Count vehicles from the query + * + * @param Select $select Original select + * @param ?AutosList $filters Filters + */ + private function proceedCount(Select $select, ?AutosList $filters): void + { + $countSelect = clone $select; + $countSelect->reset($countSelect::COLUMNS); + $countSelect->reset($countSelect::JOINS); + $countSelect->reset($countSelect::ORDER); + $countSelect->columns( + [ + 'count' => new Expression('count(DISTINCT a.' . Auto::PK . ')') + ] + ); + + $this->count = (int)$this->zdb->execute($countSelect)->current()['count']; + if ($this->count > 0 && $filters !== null) { + $filters->setCounter($this->count); + } + } +} diff --git a/tests/GaletteAuto/tests/units/Auto.php b/tests/GaletteAuto/tests/units/Auto.php index b553c4b..4c7c9b2 100644 --- a/tests/GaletteAuto/tests/units/Auto.php +++ b/tests/GaletteAuto/tests/units/Auto.php @@ -78,6 +78,7 @@ public function testCrud(): void $this->logSuperAdmin(); $access = new \GaletteAuto\VehicleAccess($this->zdb, $this->login, $this->preferences); + $vehicles = new \GaletteAuto\Repository\Vehicles($this->plugins, $this->zdb, $this->login, $this->history); $auto = new \GaletteAuto\Auto($this->plugins, $this->zdb); $data = []; @@ -142,9 +143,7 @@ public function testCrud(): void $this->assertSame([], $auto->getErrors()); $this->assertTrue($check); - $stored = $auto->store(true); - $this->assertEquals([], $auto->getErrors()); - $this->assertTrue($stored); + $vehicles->store($auto); $auto_id = $auto->getId(); //check history @@ -204,9 +203,9 @@ public function testCrud(): void $this->assertSame([], $auto->getErrors()); $this->assertTrue($check); - $stored = $auto->store(); - $this->assertEquals([], $auto->getErrors()); - $this->assertTrue($stored); + //history is sorted on a date with seconds: entries of the same second have no order + sleep(1); + $vehicles->store($auto); //check history $history = new \GaletteAuto\History($this->zdb); @@ -240,24 +239,21 @@ public function testCrud(): void $this->assertSame([], $auto->getErrors()); $this->assertTrue($check); - $stored = $auto->store(true); - $this->assertEquals([], $auto->getErrors()); - $this->assertTrue($stored); + $vehicles->store($auto); $auto2_id = $auto->getId(); $this->assertTrue($history->load($auto2_id)); $this->assertCount(1, $history->getEntries()); - $autos = new \GaletteAuto\Autos($this->plugins, $this->zdb); - $this->assertCount(2, $autos->getList()); //sorted by name $this->assertSame( ['My car', 'Titine'], - array_map(fn($car) => $car->getName(), $autos->getList(true)) + array_map(fn($car) => $car->getName(), $vehicles->getList()) ); + $this->assertSame(2, $vehicles->getCount()); - $this->assertTrue($autos->removeVehicles([$auto_id])); - $this->assertCount(1, $autos->getList()); + $vehicles->remove([$auto_id]); + $this->assertCount(1, $vehicles->getList()); $this->expectNoLogEntry(); $this->assertFalse($auto->load($auto_id)); $this->expectLogEntry( From 48d0bfef358c7af223fb74bde31a04b5efed22f1 Mon Sep 17 00:00:00 2001 From: Johan Cwiklinski Date: Sat, 26 Sep 2026 14:54:38 +0200 Subject: [PATCH 6/6] Inject login in plugin class, keep posted property value in session instead of the entity --- .../Controllers/Crud/PropertiesController.php | 13 +++++++------ lib/GaletteAuto/PluginGaletteAuto.php | 8 ++++---- .../tests/units/PropertiesController.php | 6 ++++++ 3 files changed, 17 insertions(+), 10 deletions(-) diff --git a/lib/GaletteAuto/Controllers/Crud/PropertiesController.php b/lib/GaletteAuto/Controllers/Crud/PropertiesController.php index 7cf6b79..5c5b7b8 100644 --- a/lib/GaletteAuto/Controllers/Crud/PropertiesController.php +++ b/lib/GaletteAuto/Controllers/Crud/PropertiesController.php @@ -286,10 +286,11 @@ public function propertyEdit(Response $response, string $property, ?int $id = nu ); } - $session_oname = 'auto_' . $property; - if ($this->session->$session_oname !== null) { - $object = $this->session->$session_oname; - $this->session->$session_oname = null; + //value from a failed submission + $session_oname = 'auto_' . $property . '_data'; + if (isset($this->session->$session_oname)) { + $object->setValue((string)$this->session->$session_oname); + unset($this->session->$session_oname); } $params = [ @@ -378,8 +379,8 @@ public function doPropertyEdit( if (count($error_detected) > 0) { //store entity in session - $session_oname = 'auto_' . $property; - $this->session->$session_oname = $object; + $session_oname = 'auto_' . $property . '_data'; + $this->session->$session_oname = (string)($value ?? ''); if ($is_new) { $route = $this->routeparser->urlFor('propertyAdd', ['property' => $property]); } else { diff --git a/lib/GaletteAuto/PluginGaletteAuto.php b/lib/GaletteAuto/PluginGaletteAuto.php index 2d31ccf..373d082 100644 --- a/lib/GaletteAuto/PluginGaletteAuto.php +++ b/lib/GaletteAuto/PluginGaletteAuto.php @@ -31,6 +31,8 @@ class PluginGaletteAuto extends GalettePlugin implements MenuProviderInterface, { #[Inject] private readonly Db $zdb; //@phpstan-ignore property.uninitializedReadonly,property.onlyRead (injected from DI) + #[Inject] + private readonly Login $login; //@phpstan-ignore property.uninitializedReadonly,property.onlyRead (injected from DI) /** * Get plugins menus @@ -39,8 +41,7 @@ class PluginGaletteAuto extends GalettePlugin implements MenuProviderInterface, */ public function getMenus(): array { - /** @var Login $login */ - global $login; + $login = $this->login; $menus = []; if ($login->isLogged()) { @@ -165,8 +166,7 @@ public function getPublicPageLabel(string $id): string */ public function getMyDashboards(): array { - /** @var Login $login */ - global $login; + $login = $this->login; if ($login->isSuperAdmin()) { return []; diff --git a/tests/GaletteAuto/Controllers/tests/units/PropertiesController.php b/tests/GaletteAuto/Controllers/tests/units/PropertiesController.php index f05e1e3..29bf254 100644 --- a/tests/GaletteAuto/Controllers/tests/units/PropertiesController.php +++ b/tests/GaletteAuto/Controllers/tests/units/PropertiesController.php @@ -95,6 +95,12 @@ public function testAddError(): void $test_response->getHeaders() ); $this->expectFlashData(['error_detected' => ['- You must provide a value!']]); + //posted value is kept in session, not the entity + $this->assertSame('', $this->session->auto_color_data); + + $test_response = $this->app->handle($this->createRequest('propertyAdd', ['property' => 'color'])); + $this->expectOK($test_response); + $this->assertFalse(isset($this->session->auto_color_data)); } /**