From 30d0eb1764b4c4b0b812412802262a94a51d2553 Mon Sep 17 00:00:00 2001 From: Johan Cwiklinski Date: Sat, 26 Sep 2026 18:16:33 +0200 Subject: [PATCH 1/3] Reduce vehicle properties to their constants and labels, drop controller switches --- lib/GaletteAuto/AbstractObject.php | 205 ++++++++--------- lib/GaletteAuto/Body.php | 75 ++++--- lib/GaletteAuto/Brand.php | 87 +++++--- lib/GaletteAuto/Color.php | 75 ++++--- .../Controllers/Crud/PropertiesController.php | 206 +++--------------- lib/GaletteAuto/Finition.php | 75 ++++--- lib/GaletteAuto/State.php | 75 ++++--- lib/GaletteAuto/Transmission.php | 75 ++++--- templates/default/object_list.html.twig | 2 +- tests/GaletteAuto/tests/units/Body.php | 2 +- tests/GaletteAuto/tests/units/Brand.php | 2 +- tests/GaletteAuto/tests/units/Color.php | 2 +- tests/GaletteAuto/tests/units/Finition.php | 2 +- tests/GaletteAuto/tests/units/State.php | 2 +- .../GaletteAuto/tests/units/Transmission.php | 2 +- 15 files changed, 430 insertions(+), 457 deletions(-) diff --git a/lib/GaletteAuto/AbstractObject.php b/lib/GaletteAuto/AbstractObject.php index e051035..7ce31b1 100644 --- a/lib/GaletteAuto/AbstractObject.php +++ b/lib/GaletteAuto/AbstractObject.php @@ -25,10 +25,25 @@ */ abstract class AbstractObject { - private string $table; - private string $pk; - private string $field; - private string $name; + public const string TABLE = ''; + public const string PK = ''; + public const string FIELD = ''; + /** Name of the list route */ + public const string LIST_ROUTE = ''; + + /** + * Properties classes, by route property name + * + * @var array> + */ + private const array CLASSES = [ + Body::FIELD => Body::class, + Brand::FIELD => Brand::class, + Color::FIELD => Color::class, + Finition::FIELD => Finition::class, + State::FIELD => State::class, + Transmission::FIELD => Transmission::class, + ]; protected Db $zdb; protected ?int $id = null; @@ -40,25 +55,29 @@ abstract class AbstractObject /** * Default constructor * - * @param Db $zdb Database instance - * @param string $table table name - * @param string $pk primary key field - * @param string $field main field name - * @param string $name name - * @param ?int $id id to load. Defaults to null + * @param Db $zdb Database instance + * @param ?int $id id to load. Defaults to null */ - public function __construct(Db $zdb, string $table, string $pk, string $field, string $name, ?int $id = null) + final public function __construct(Db $zdb, ?int $id = null) { $this->zdb = $zdb; - $this->table = AUTO_PREFIX . $table; - $this->pk = $pk; - $this->field = $field; - $this->name = $name; if (is_int($id)) { $this->load($id); } } + /** + * Get a property instance from its route name + * + * @param Db $zdb Database instance + * @param string $property Route property name + */ + public static function fromPropertyName(Db $zdb, string $property): self + { + $class = self::getClassForPropName($property); + return new $class($zdb); + } + /** * Get the list * @@ -76,7 +95,7 @@ public function getList(): array return $list; } catch (\Exception $e) { Analog::log( - '[' . get_class($this) . '] Cannot load ' . $this->name + '[' . get_class($this) . '] Cannot load ' . static::TABLE . ' list | ' . $e->getMessage(), Analog::ERROR ); @@ -92,10 +111,10 @@ public function getList(): array public function load(int $id): bool { try { - $select = $this->zdb->select($this->table); + $select = $this->zdb->select(AUTO_PREFIX . static::TABLE); $select->where( [ - $this->pk => $id + static::PK => $id ] ); @@ -108,7 +127,7 @@ public function load(int $id): bool return true; } catch (\Exception $e) { Analog::log( - '[' . get_class($this) . '] Cannot load ' . $this->name + '[' . get_class($this) . '] Cannot load ' . static::TABLE . ' from id `' . $id . '` | ' . $e->getMessage(), Analog::ERROR ); @@ -123,8 +142,8 @@ public function load(int $id): bool */ public function loadFromRow(ArrayObject $row): self { - $this->id = (int)$row[$this->pk]; - $this->value = (string)$row[$this->field]; + $this->id = (int)$row[static::PK]; + $this->value = (string)$row[static::FIELD]; return $this; } @@ -137,23 +156,23 @@ public function store(bool $new = false): bool { try { $values = [ - $this->field => $this->value + static::FIELD => $this->value ]; if ($new) { - $insert = $this->zdb->insert($this->table); + $insert = $this->zdb->insert(AUTO_PREFIX . static::TABLE); $insert->values($values); $this->zdb->execute($insert); /** @phpstan-ignore-next-line */ $this->id = (int)$this->zdb->driver->getLastGeneratedValue( $this->zdb->isPostgres() - ? PREFIX_DB . $this->table . '_id_seq' + ? PREFIX_DB . AUTO_PREFIX . static::TABLE . '_id_seq' : null ); } else { - $update = $this->zdb->update($this->table); + $update = $this->zdb->update(AUTO_PREFIX . static::TABLE); $update->set($values)->where( [ - $this->pk => $this->id + static::PK => $this->id ] ); $this->zdb->execute($update); @@ -161,7 +180,7 @@ public function store(bool $new = false): bool return true; } catch (\Exception $e) { Analog::log( - '[' . get_class($this) . '] Cannot store ' . $this->name + '[' . get_class($this) . '] Cannot store ' . static::TABLE . ' values `' . ($this->id ?? '') . '`, `' . $this->value . '` | ' . $e->getMessage(), Analog::WARNING @@ -178,13 +197,13 @@ public function store(bool $new = false): bool public function delete(array $ids): bool { try { - $delete = $this->zdb->delete($this->table); - $delete->where->in($this->pk, $ids); + $delete = $this->zdb->delete(AUTO_PREFIX . static::TABLE); + $delete->where->in(static::PK, $ids); $this->zdb->execute($delete); return true; } catch (\Exception $e) { Analog::log( - '[' . get_class($this) . '] Cannot delete ' . $this->name + '[' . get_class($this) . '] Cannot delete ' . static::TABLE . ' from ids `' . implode(' - ', $ids) . '` | ' . $e->getMessage(), Analog::WARNING ); @@ -208,10 +227,55 @@ public function setFilters(PropertiesList $filters): self */ abstract public function getFieldLabel(): string; + /** + * Get list page title + */ + abstract public function getListTitle(): string; + + /** + * Get add button text + */ + abstract public function getAddText(): string; + + /** + * Get localized count + * + * @param int $count Count + */ + abstract public function getCountLabel(int $count): string; + + /** + * Get removal success message + * + * @param int $count Removed records count + */ + abstract public function getRemovedMessage(int $count): string; + + /** + * Get message when removal is refused because the record is in use + */ + abstract public function getInUseMessage(): string; + + /** + * Get removal error message + */ + abstract public function getRemoveErrorMessage(): string; + + /** + * Whether records have a details page + */ + public function hasDetails(): bool + { + return false; + } + /** * Get property route name */ - abstract public function getRouteName(): string; + public function getRouteName(): string + { + return static::FIELD; + } /** * Get record ID @@ -245,7 +309,7 @@ public function setValue(string $value): self */ public function getPk(): string { - return $this->pk; + return static::PK; } /** @@ -253,74 +317,32 @@ public function getPk(): string */ public function getField(): string { - return $this->field; + return static::FIELD; } /** * Get list route * * @param RouteParser $routeparser Route parser instance - * @param string $property Property name */ - public static function getListRoute(RouteParser $routeparser, string $property): string + public static function getListRoute(RouteParser $routeparser): string { - $route = null; - switch ($property) { - case 'color': - $route = $routeparser->urlFor('colorsList'); - break; - case 'state': - $route = $routeparser->urlFor('statesList'); - break; - case 'finition': - $route = $routeparser->urlFor('finitionsList'); - break; - case 'body': - $route = $routeparser->urlFor('bodiesList'); - break; - case 'transmission': - $route = $routeparser->urlFor('transmissionsList'); - break; - case 'brand': - $route = $routeparser->urlFor('brandsList'); - break; - default: - throw new \RuntimeException('Unknown property ' . $property); - } - return $route; + return $routeparser->urlFor(static::LIST_ROUTE); } /** - * Get object name from route property + * Get object class name from route property * * @param string $property Route property + * + * @return class-string */ public static function getClassForPropName(string $property): string { - $classname = '\GaletteAuto\\'; - switch ($property) { - case 'brand': - $classname .= 'Brand'; - break; - case 'color': - $classname .= 'Color'; - break; - case 'state': - $classname .= 'State'; - break; - case 'finition': - $classname .= 'Finition'; - break; - case 'body': - $classname .= 'Body'; - break; - case 'transmission': - $classname .= 'Transmission'; - break; - default: - throw new \RuntimeException('Unknown property ' . $property); + if (!isset(self::CLASSES[$property])) { + throw new \RuntimeException('Unknown property ' . $property); } - return $classname; + return self::CLASSES[$property]; } /** @@ -331,8 +353,8 @@ public static function getClassForPropName(string $property): string private function buildSelect(): Select { try { - $select = $this->zdb->select($this->table); - $select->order([$this->field . ' ASC', $this->pk . ' ASC']); + $select = $this->zdb->select(AUTO_PREFIX . static::TABLE); + $select->order([static::FIELD . ' ASC', static::PK . ' ASC']); if (isset($this->filters)) { $this->filters->setLimits($select); } @@ -364,7 +386,6 @@ private function proceedCount(Select $select): void $countSelect->reset($countSelect::OFFSET); $countSelect->columns( [ - //@phpstan-ignore-next-line static::PK => new Expression('COUNT(' . static::PK . ')') ] ); @@ -372,7 +393,6 @@ private function proceedCount(Select $select): void $results = $this->zdb->execute($countSelect); $result = $results->current(); - //@phpstan-ignore-next-line $k = static::PK; $this->count = (int)$result->$k; @@ -401,15 +421,6 @@ public function getCount(): int */ public function displayCount(): string { - return str_replace( - '%count', - (string)$this->getCount(), - $this->getLocalizedCount() - ); + return $this->getCountLabel($this->getCount()); } - - /** - * Get localized count string for object list - */ - abstract protected function getLocalizedCount(): string; } diff --git a/lib/GaletteAuto/Body.php b/lib/GaletteAuto/Body.php index 92fbfab..3d0078e 100644 --- a/lib/GaletteAuto/Body.php +++ b/lib/GaletteAuto/Body.php @@ -10,8 +10,6 @@ namespace GaletteAuto; -use Galette\Core\Db; - /** * Automobile Bodies class for galette Auto plugin * @@ -22,53 +20,72 @@ class Body extends AbstractObject public const string TABLE = 'bodies'; public const string PK = 'id_body'; public const string FIELD = 'body'; - public const string NAME = 'bodies'; + public const string LIST_ROUTE = 'bodiesList'; /** - * Default constructor - * - * @param Db $zdb Database instance - * @param ?int $id body's id to load. Defaults to null + * Get field label */ - public function __construct(Db $zdb, ?int $id = null) + public function getFieldLabel(): string { - parent::__construct( - $zdb, - self::TABLE, - self::PK, - self::FIELD, - self::NAME, - $id - ); + return _T('Body', 'auto'); } /** - * Get field label + * Get list page title */ - public function getFieldLabel(): string + public function getListTitle(): string { - return _T('Body', 'auto'); + return _T("Bodies list", "auto"); } /** - * Get property route name + * Get add button text */ - public function getRouteName(): string + public function getAddText(): string { - return 'body'; + return _T("Add new body", "auto"); } + /** + * Get localized count + * + * @param int $count Count + */ + public function getCountLabel(int $count): string + { + return str_replace( + '%count', + (string)$count, + _Tn('%count body', '%count bodies', $count, 'auto') + ); + } /** - * Get localized count string for object list + * Get removal success message + * + * @param int $count Removed records count */ - protected function getLocalizedCount(): string + public function getRemovedMessage(int $count): string { - return _Tn( - '%count body', - '%count bodies', - $this->getCount(), - 'auto' + return sprintf( + _Tn('%1$s body has been successfully deleted.', '%1$s bodies have been successfully deleted.', $count, 'auto'), + $count ); } + + /** + * Get message when removal is refused because the record is in use + */ + public function getInUseMessage(): string + { + return _T('This body is used by one or more vehicles, it cannot be deleted.', 'auto'); + } + + /** + * Get removal error message + */ + public function getRemoveErrorMessage(): string + { + return _T('An error occurred trying to remove body :/', 'auto'); + } } diff --git a/lib/GaletteAuto/Brand.php b/lib/GaletteAuto/Brand.php index 8290ff9..019fb96 100644 --- a/lib/GaletteAuto/Brand.php +++ b/lib/GaletteAuto/Brand.php @@ -10,71 +10,90 @@ namespace GaletteAuto; -use Galette\Core\Db; - /** * Automobile Brands class for galette Auto plugin * - * @category Plugins - * @name AutoBrands * @author Johan Cwiklinski - * @copyright 2009-2014 The Galette Team - * @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 */ class Brand extends AbstractObject { public const string TABLE = 'brands'; public const string PK = 'id_brand'; public const string FIELD = 'brand'; - public const string NAME = 'brands'; + public const string LIST_ROUTE = 'brandsList'; /** - * Default constructor + * Get field label + */ + public function getFieldLabel(): string + { + return _T('Brand', 'auto'); + } + + /** + * Get list page title + */ + public function getListTitle(): string + { + return _T("Brands list", "auto"); + } + + /** + * Get add button text + */ + public function getAddText(): string + { + return _T("Add new brand", "auto"); + } + + /** + * Get localized count * - * @param Db $zdb Database instance - * @param ?int $id brand's id to load. Defaults to null + * @param int $count Count */ - public function __construct(Db $zdb, ?int $id = null) + public function getCountLabel(int $count): string { - parent::__construct( - $zdb, - self::TABLE, - self::PK, - self::FIELD, - self::NAME, - $id + return str_replace( + '%count', + (string)$count, + _Tn('%count brand', '%count brands', $count, 'auto') ); } /** - * Get field label + * Get removal success message + * + * @param int $count Removed records count */ - public function getFieldLabel(): string + public function getRemovedMessage(int $count): string { - return _T('Brand', 'auto'); + return sprintf( + _Tn('%1$s brand has been successfully deleted.', '%1$s brands have been successfully deleted.', $count, 'auto'), + $count + ); } /** - * Get property route name + * Get message when removal is refused because the record is in use */ - public function getRouteName(): string + public function getInUseMessage(): string { - return 'brand'; + return _T('This brand is used by one or more vehicles, it cannot be deleted.', 'auto'); } + /** + * Get removal error message + */ + public function getRemoveErrorMessage(): string + { + return _T('An error occurred trying to remove brand :/', 'auto'); + } /** - * Get localized count string for object list + * Brands have a page listing their models */ - protected function getLocalizedCount(): string + public function hasDetails(): bool { - return _Tn( - '%count brand', - '%count brands', - $this->getCount(), - 'auto' - ); + return true; } } diff --git a/lib/GaletteAuto/Color.php b/lib/GaletteAuto/Color.php index 8e826a6..3571f57 100644 --- a/lib/GaletteAuto/Color.php +++ b/lib/GaletteAuto/Color.php @@ -10,8 +10,6 @@ namespace GaletteAuto; -use Galette\Core\Db; - /** * Automobile Colors class for galette Auto plugin * @@ -22,53 +20,72 @@ class Color extends AbstractObject public const string TABLE = 'colors'; public const string PK = 'id_color'; public const string FIELD = 'color'; - public const string NAME = 'colors'; + public const string LIST_ROUTE = 'colorsList'; /** - * Default constructor - * - * @param Db $zdb Database instance - * @param ?int $id state's id to load. Defaults to null + * Get field label */ - public function __construct(Db $zdb, ?int $id = null) + public function getFieldLabel(): string { - parent::__construct( - $zdb, - self::TABLE, - self::PK, - self::FIELD, - self::NAME, - $id - ); + return _T('Color', 'auto'); } /** - * Get field label + * Get list page title */ - public function getFieldLabel(): string + public function getListTitle(): string { - return _T('Color', 'auto'); + return _T("Colors list", "auto"); } /** - * Get property route name + * Get add button text */ - public function getRouteName(): string + public function getAddText(): string { - return 'color'; + return _T("Add new color", "auto"); } + /** + * Get localized count + * + * @param int $count Count + */ + public function getCountLabel(int $count): string + { + return str_replace( + '%count', + (string)$count, + _Tn('%count color', '%count colors', $count, 'auto') + ); + } /** - * Get localized count string for object list + * Get removal success message + * + * @param int $count Removed records count */ - protected function getLocalizedCount(): string + public function getRemovedMessage(int $count): string { - return _Tn( - '%count color', - '%count colors', - $this->getCount(), - 'auto' + return sprintf( + _Tn('%1$s color has been successfully deleted.', '%1$s colors have been successfully deleted.', $count, 'auto'), + $count ); } + + /** + * Get message when removal is refused because the record is in use + */ + public function getInUseMessage(): string + { + return _T('This color is used by one or more vehicles, it cannot be deleted.', 'auto'); + } + + /** + * Get removal error message + */ + public function getRemoveErrorMessage(): string + { + return _T('An error occurred trying to remove color :/', 'auto'); + } } diff --git a/lib/GaletteAuto/Controllers/Crud/PropertiesController.php b/lib/GaletteAuto/Controllers/Crud/PropertiesController.php index 5c5b7b8..24b1a3f 100644 --- a/lib/GaletteAuto/Controllers/Crud/PropertiesController.php +++ b/lib/GaletteAuto/Controllers/Crud/PropertiesController.php @@ -50,7 +50,7 @@ public function brandsList( ?string $option = null, string|int|null $value = null ): Response { - return $this->propertiesList($request, $response, 'brands', $option, $value); + return $this->propertiesList($request, $response, new Brand($this->zdb), $option, $value); } /** @@ -65,7 +65,7 @@ public function colorsList( ?string $option = null, int|string|null $value = null ): Response { - return $this->propertiesList($request, $response, 'colors', $option, $value); + return $this->propertiesList($request, $response, new Color($this->zdb), $option, $value); } /** @@ -80,7 +80,7 @@ public function statesList( ?string $option = null, string|int|null $value = null ): Response { - return $this->propertiesList($request, $response, 'states', $option, $value); + return $this->propertiesList($request, $response, new State($this->zdb), $option, $value); } /** @@ -95,7 +95,7 @@ public function finitionsList( ?string $option = null, string|int|null $value = null ): Response { - return $this->propertiesList($request, $response, 'finitions', $option, $value); + return $this->propertiesList($request, $response, new Finition($this->zdb), $option, $value); } /** @@ -110,7 +110,7 @@ public function bodiesList( ?string $option = null, string|int|null $value = null ): Response { - return $this->propertiesList($request, $response, 'bodies', $option, $value); + return $this->propertiesList($request, $response, new Body($this->zdb), $option, $value); } /** @@ -125,61 +125,25 @@ public function transmissionsList( ?string $option = null, string|int|null $value = null ): Response { - return $this->propertiesList($request, $response, 'transmissions', $option, $value); + return $this->propertiesList($request, $response, new Transmission($this->zdb), $option, $value); } /** * List properties * - * @param string $property Property name - * @param string|null $option One of 'page' or 'order' - * @param string|int|null $value Value of the option + * @param AbstractObject $obj Property instance + * @param string|null $option One of 'page' or 'order' + * @param string|int|null $value Value of the option */ protected function propertiesList( Request $request, Response $response, - string $property, + AbstractObject $obj, ?string $option = null, string|int|null $value = null ): Response { $get = $request->getQueryParams(); - switch ($property) { - case 'colors': - $obj = new Color($this->zdb); - $title = _T("Colors list", "auto"); - $add_text = _T("Add new color", "auto"); - break; - case 'states': - $obj = new State($this->zdb); - $title = _T("States list", "auto"); - $add_text = _T("Add new state", "auto"); - break; - case 'finitions': - $obj = new Finition($this->zdb); - $title = _T("Finitions list", "auto"); - $add_text = _T("Add new finition", "auto"); - break; - case 'bodies': - $obj = new Body($this->zdb); - $title = _T("Bodies list", "auto"); - $add_text = _T("Add new body", "auto"); - break; - case 'transmissions': - $obj = new Transmission($this->zdb); - $title = _T("Transmissions list", "auto"); - $add_text = _T("Add new transmission", "auto"); - break; - case 'brands': - $obj = new Brand($this->zdb); - $title = _T("Brands list", "auto"); - $add_text = _T("Add new brand", "auto"); - $can_show = true; - break; - default: - throw new \RuntimeException('Unknown property ' . $property); - } - $filters = $this->getFilters($obj); if (isset($get['nbshow']) && is_numeric($get['nbshow'])) { $filters->show = $get['nbshow']; @@ -198,11 +162,10 @@ protected function propertiesList( $this->saveFilters($obj, $filters); $params = [ - 'page_title' => $title, + 'page_title' => $obj->getListTitle(), 'list' => $obj->getList(), - 'set' => $property, 'field_name' => $obj->getFieldLabel(), - 'add_text' => $add_text, + 'add_text' => $obj->getAddText(), 'obj' => $obj, 'require_dialog' => true ]; @@ -210,10 +173,6 @@ protected function propertiesList( //assign pagination variables to the template and add pagination links $filters->setViewPagination($this->routeparser, $this->view, false); - if (isset($can_show)) { - $params['show'] = $can_show; - } - // display page $this->view->render( $response, @@ -231,7 +190,7 @@ protected function propertiesList( public function filter(Request $request, Response $response, string $property): Response { $post = $request->getParsedBody(); - $class = '\GaletteAuto\\' . ucwords($property); + $class = AbstractObject::getClassForPropName($property); $filters = $this->getFilters($class); if (isset($post['clear_filter'])) { @@ -248,7 +207,7 @@ public function filter(Request $request, Response $response, string $property): ->withStatus(301) ->withHeader( 'Location', - $class::getListRoute($this->routeparser, $property) + $class::getListRoute($this->routeparser) ); } @@ -273,8 +232,7 @@ public function propertyEdit(Response $response, string $property, ?int $id = nu { $is_new = ($action === 'add'); - $classname = AbstractObject::getClassForPropName($property); - $object = new $classname($this->zdb); + $object = AbstractObject::fromPropertyName($this->zdb, $property); if ($is_new) { $title = _T("New", "auto"); } else { @@ -336,8 +294,7 @@ public function doPropertyEdit( ?int $id = null, string $action = 'edit', ): Response { - $classname = AbstractObject::getClassForPropName($property); - $object = new $classname($this->zdb); + $object = AbstractObject::fromPropertyName($this->zdb, $property); $post = $request->getParsedBody(); $is_new = ($action === 'add'); @@ -375,7 +332,7 @@ public function doPropertyEdit( } } - $route = AbstractObject::getListRoute($this->routeparser, $property); + $route = $object::getListRoute($this->routeparser); if (count($error_detected) > 0) { //store entity in session @@ -414,8 +371,7 @@ public function doPropertyEdit( */ public function propertyShow(Response $response, string $property, int $id): Response { - $classname = AbstractObject::getClassForPropName($property); - $object = new $classname($this->zdb); + $object = AbstractObject::fromPropertyName($this->zdb, $property); $object->load($id); $title = str_replace( '%s', @@ -455,11 +411,10 @@ public function propertyShow(Response $response, string $property, int $id): Res */ public function removeProperty(Request $request, Response $response, string $property, int $id): Response { - $classname = AbstractObject::getClassForPropName($property); - $object = new $classname($this->zdb); + $object = AbstractObject::fromPropertyName($this->zdb, $property); $object->load($id); - $route = AbstractObject::getListRoute($this->routeparser, $property); + $route = $object::getListRoute($this->routeparser); $data = [ 'id' => $id, @@ -481,7 +436,7 @@ public function removeProperty(Request $request, Response $response, string $pro ), 'form_url' => $this->routeparser->urlFor( 'doRemoveProperty', - ['property' => $property, 'id' => $object->getId()] + ['property' => $property, 'id' => (string)$id] ), 'cancel_uri' => $route, 'data' => $data @@ -517,117 +472,22 @@ public function doRemoveProperty(Request $request, Response $response, string $p $ids = $post['id']; } - $classname = AbstractObject::getClassForPropName($property); - $object = new $classname($this->zdb); + $object = AbstractObject::fromPropertyName($this->zdb, $property); try { $object->delete($ids); - - switch ($property) { - case 'colors': - case 'color': - $message = _Tn('%1$s color has been successfully deleted.', '%1$s colors have been successfully deleted.', count($ids), 'auto'); - break; - case 'states': - case 'state': - $message = _Tn('%1$s state has been successfully deleted.', '%1$s states have been successfully deleted.', count($ids), 'auto'); - break; - case 'finitions': - case 'finition': - $message = _Tn('%1$s finition has been successfully deleted.', '%1$s finitions have been successfully deleted.', count($ids), 'auto'); - break; - case 'bodies': - case 'body': - $message = _Tn('%1$s body has been successfully deleted.', '%1$s bodies have been successfully deleted.', count($ids), 'auto'); - break; - case 'transmissions': - case 'transmission': - $message = _Tn('%1$s transmission has been successfully deleted.', '%1$s transmissions have been successfully deleted.', count($ids), 'auto'); - break; - case 'brands': - case 'brand': - $message = _Tn('%1$s brand has been successfully deleted.', '%1$s brands have been successfully deleted.', count($ids), 'auto'); - break; - default: - throw new \RuntimeException('Unknown property ' . $property); - } - $this->flash->addMessage( 'success_detected', - sprintf($message, count($ids)) + $object->getRemovedMessage(count($ids)) ); - $success = true; } catch (\Throwable $e) { - if ($this->zdb->isForeignKeyException($e)) { - switch ($property) { - case 'colors': - case 'color': - $message = _T('This color is used by one or more vehicles, it cannot be deleted.', 'auto'); - break; - case 'states': - case 'state': - $message = _T('This state is used by one or more vehicles, it cannot be deleted.', 'auto'); - break; - case 'finitions': - case 'finition': - $message = _T('This finition is used by one or more vehicles, it cannot be deleted.', 'auto'); - break; - case 'bodies': - case 'body': - $message = _T('This body is used by one or more vehicles, it cannot be deleted.', 'auto'); - break; - case 'transmissions': - case 'transmission': - $message = _T('This transmission is used by one or more vehicles, it cannot be deleted.', 'auto'); - break; - case 'brands': - case 'brand': - $message = _T('This brand is used by one or more vehicles, it cannot be deleted.', 'auto'); - break; - default: - throw new \RuntimeException('Unknown property ' . $property); - } - - $this->flash->addMessage( - 'error_detected', - $message - ); - } else { - switch ($property) { - case 'colors': - case 'color': - $message = _T('An error occurred trying to remove color :/', 'auto'); - break; - case 'states': - case 'state': - $message = _T('An error occurred trying to remove state :/', 'auto'); - break; - case 'finitions': - case 'finition': - $message = _T('An error occurred trying to remove finition :/', 'auto'); - break; - case 'bodies': - case 'body': - $message = _T('An error occurred trying to remove body :/', 'auto'); - break; - case 'transmissions': - case 'transmission': - $message = _T('An error occurred trying to remove transmission :/', 'auto'); - break; - case 'brands': - case 'brand': - $message = _T('An error occurred trying to remove brand :/', 'auto'); - break; - default: - throw new \RuntimeException('Unknown property ' . $property); - } - - $this->flash->addMessage( - 'error_detected', - $message - ); - } + $this->flash->addMessage( + 'error_detected', + $this->zdb->isForeignKeyException($e) + ? $object->getInUseMessage() + : $object->getRemoveErrorMessage() + ); } } @@ -648,11 +508,10 @@ public function doRemoveProperty(Request $request, Response $response, string $p /** * Get filters * - * @param AbstractObject|string $class Class name or instance + * @param AbstractObject|class-string $class Class name or instance */ protected function getFilters(AbstractObject|string $class): PropertiesList { - /** @phpstan-ignore-next-line */ $filter_name = 'filter_auto' . $class::FIELD; return $this->session->$filter_name ?? new PropertiesList(); } @@ -660,12 +519,11 @@ protected function getFilters(AbstractObject|string $class): PropertiesList /** * Save filters * - * @param AbstractObject|string $class Class name or instance - * @param PropertiesList $filters Filters instance + * @param AbstractObject|class-string $class Class name or instance + * @param PropertiesList $filters Filters instance */ protected function saveFilters(AbstractObject|string $class, PropertiesList $filters): void { - /** @phpstan-ignore-next-line */ $filter_name = 'filter_auto' . $class::FIELD; $this->session->$filter_name = $filters; } diff --git a/lib/GaletteAuto/Finition.php b/lib/GaletteAuto/Finition.php index 0341dce..532506f 100644 --- a/lib/GaletteAuto/Finition.php +++ b/lib/GaletteAuto/Finition.php @@ -10,8 +10,6 @@ namespace GaletteAuto; -use Galette\Core\Db; - /** * Automobile Finitions class for galette Auto plugin * @@ -22,53 +20,72 @@ class Finition extends AbstractObject public const string TABLE = 'finitions'; public const string PK = 'id_finition'; public const string FIELD = 'finition'; - public const string NAME = 'finitions'; + public const string LIST_ROUTE = 'finitionsList'; /** - * Default constructor - * - * @param Db $zdb Database instance - * @param ?int $id finition's id to load. Defaults to null + * Get field label */ - public function __construct(Db $zdb, ?int $id = null) + public function getFieldLabel(): string { - parent::__construct( - $zdb, - self::TABLE, - self::PK, - self::FIELD, - self::NAME, - $id - ); + return _T('Finition', 'auto'); } /** - * Get field label + * Get list page title */ - public function getFieldLabel(): string + public function getListTitle(): string { - return _T('Finition', 'auto'); + return _T("Finitions list", "auto"); } /** - * Get property route name + * Get add button text */ - public function getRouteName(): string + public function getAddText(): string { - return 'finition'; + return _T("Add new finition", "auto"); } + /** + * Get localized count + * + * @param int $count Count + */ + public function getCountLabel(int $count): string + { + return str_replace( + '%count', + (string)$count, + _Tn('%count finition', '%count finitions', $count, 'auto') + ); + } /** - * Get localized count string for object list + * Get removal success message + * + * @param int $count Removed records count */ - protected function getLocalizedCount(): string + public function getRemovedMessage(int $count): string { - return _Tn( - '%count finition', - '%count finitions', - $this->getCount(), - 'auto' + return sprintf( + _Tn('%1$s finition has been successfully deleted.', '%1$s finitions have been successfully deleted.', $count, 'auto'), + $count ); } + + /** + * Get message when removal is refused because the record is in use + */ + public function getInUseMessage(): string + { + return _T('This finition is used by one or more vehicles, it cannot be deleted.', 'auto'); + } + + /** + * Get removal error message + */ + public function getRemoveErrorMessage(): string + { + return _T('An error occurred trying to remove finition :/', 'auto'); + } } diff --git a/lib/GaletteAuto/State.php b/lib/GaletteAuto/State.php index b2e81d7..631651e 100644 --- a/lib/GaletteAuto/State.php +++ b/lib/GaletteAuto/State.php @@ -10,8 +10,6 @@ namespace GaletteAuto; -use Galette\Core\Db; - /** * Automobile States class for galette Auto plugin * @@ -22,53 +20,72 @@ class State extends AbstractObject public const string TABLE = 'states'; public const string PK = 'id_state'; public const string FIELD = 'state'; - public const string NAME = 'states'; + public const string LIST_ROUTE = 'statesList'; /** - * Default constructor - * - * @param Db $zdb Database instance - * @param ?int $id state's id to load. Defaults to null + * Get field label */ - public function __construct(Db $zdb, ?int $id = null) + public function getFieldLabel(): string { - parent::__construct( - $zdb, - self::TABLE, - self::PK, - self::FIELD, - self::NAME, - $id - ); + return _T('State', 'auto'); } /** - * Get field label + * Get list page title */ - public function getFieldLabel(): string + public function getListTitle(): string { - return _T('State', 'auto'); + return _T("States list", "auto"); } /** - * Get property route name + * Get add button text */ - public function getRouteName(): string + public function getAddText(): string { - return 'state'; + return _T("Add new state", "auto"); } + /** + * Get localized count + * + * @param int $count Count + */ + public function getCountLabel(int $count): string + { + return str_replace( + '%count', + (string)$count, + _Tn('%count state', '%count states', $count, 'auto') + ); + } /** - * Get localized count string for object list + * Get removal success message + * + * @param int $count Removed records count */ - protected function getLocalizedCount(): string + public function getRemovedMessage(int $count): string { - return _Tn( - '%count state', - '%count states', - $this->getCount(), - 'auto' + return sprintf( + _Tn('%1$s state has been successfully deleted.', '%1$s states have been successfully deleted.', $count, 'auto'), + $count ); } + + /** + * Get message when removal is refused because the record is in use + */ + public function getInUseMessage(): string + { + return _T('This state is used by one or more vehicles, it cannot be deleted.', 'auto'); + } + + /** + * Get removal error message + */ + public function getRemoveErrorMessage(): string + { + return _T('An error occurred trying to remove state :/', 'auto'); + } } diff --git a/lib/GaletteAuto/Transmission.php b/lib/GaletteAuto/Transmission.php index 74047f2..c69467a 100644 --- a/lib/GaletteAuto/Transmission.php +++ b/lib/GaletteAuto/Transmission.php @@ -10,8 +10,6 @@ namespace GaletteAuto; -use Galette\Core\Db; - /** * Automobile Transmissions class for galette Auto plugin * @@ -22,53 +20,72 @@ class Transmission extends AbstractObject public const string TABLE = 'transmissions'; public const string PK = 'id_transmission'; public const string FIELD = 'transmission'; - public const string NAME = 'transmissions'; + public const string LIST_ROUTE = 'transmissionsList'; /** - * Default constructor - * - * @param Db $zdb Database instance - * @param ?int $id transmission's id to load. Defaults to null + * Get field label */ - public function __construct(Db $zdb, ?int $id = null) + public function getFieldLabel(): string { - parent::__construct( - $zdb, - self::TABLE, - self::PK, - self::FIELD, - self::NAME, - $id - ); + return _T('Transmission', 'auto'); } /** - * Get field label + * Get list page title */ - public function getFieldLabel(): string + public function getListTitle(): string { - return _T('Transmission', 'auto'); + return _T("Transmissions list", "auto"); } /** - * Get property route name + * Get add button text */ - public function getRouteName(): string + public function getAddText(): string { - return 'transmission'; + return _T("Add new transmission", "auto"); } + /** + * Get localized count + * + * @param int $count Count + */ + public function getCountLabel(int $count): string + { + return str_replace( + '%count', + (string)$count, + _Tn('%count transmission', '%count transmissions', $count, 'auto') + ); + } /** - * Get localized count string for object list + * Get removal success message + * + * @param int $count Removed records count */ - protected function getLocalizedCount(): string + public function getRemovedMessage(int $count): string { - return _Tn( - '%count transmission', - '%count transmissions', - $this->getCount(), - 'auto' + return sprintf( + _Tn('%1$s transmission has been successfully deleted.', '%1$s transmissions have been successfully deleted.', $count, 'auto'), + $count ); } + + /** + * Get message when removal is refused because the record is in use + */ + public function getInUseMessage(): string + { + return _T('This transmission is used by one or more vehicles, it cannot be deleted.', 'auto'); + } + + /** + * Get removal error message + */ + public function getRemoveErrorMessage(): string + { + return _T('An error occurred trying to remove transmission :/', 'auto'); + } } diff --git a/templates/default/object_list.html.twig b/templates/default/object_list.html.twig index 419285f..a3ce25d 100644 --- a/templates/default/object_list.html.twig +++ b/templates/default/object_list.html.twig @@ -45,7 +45,7 @@ {{ o[field] }} diff --git a/tests/GaletteAuto/tests/units/Body.php b/tests/GaletteAuto/tests/units/Body.php index d0c57e5..f4906e9 100644 --- a/tests/GaletteAuto/tests/units/Body.php +++ b/tests/GaletteAuto/tests/units/Body.php @@ -98,6 +98,6 @@ public function testLoadError(): void */ public function testGetClassName(): void { - $this->assertSame('\\' . \GaletteAuto\Body::class, \GaletteAuto\Body::getClassForPropName('body')); + $this->assertSame(\GaletteAuto\Body::class, \GaletteAuto\AbstractObject::getClassForPropName('body')); } } diff --git a/tests/GaletteAuto/tests/units/Brand.php b/tests/GaletteAuto/tests/units/Brand.php index 89c55a8..8a51cf9 100644 --- a/tests/GaletteAuto/tests/units/Brand.php +++ b/tests/GaletteAuto/tests/units/Brand.php @@ -98,6 +98,6 @@ public function testLoadError(): void */ public function testGetClassName(): void { - $this->assertSame('\\' . \GaletteAuto\Brand::class, \GaletteAuto\Brand::getClassForPropName('brand')); + $this->assertSame(\GaletteAuto\Brand::class, \GaletteAuto\AbstractObject::getClassForPropName('brand')); } } diff --git a/tests/GaletteAuto/tests/units/Color.php b/tests/GaletteAuto/tests/units/Color.php index e825f08..f4a0572 100644 --- a/tests/GaletteAuto/tests/units/Color.php +++ b/tests/GaletteAuto/tests/units/Color.php @@ -100,6 +100,6 @@ public function testLoadError(): void */ public function testGetClassName(): void { - $this->assertSame('\\' . \GaletteAuto\Color::class, \GaletteAuto\Color::getClassForPropName('color')); + $this->assertSame(\GaletteAuto\Color::class, \GaletteAuto\AbstractObject::getClassForPropName('color')); } } diff --git a/tests/GaletteAuto/tests/units/Finition.php b/tests/GaletteAuto/tests/units/Finition.php index 55f59e3..79c5d5a 100644 --- a/tests/GaletteAuto/tests/units/Finition.php +++ b/tests/GaletteAuto/tests/units/Finition.php @@ -98,6 +98,6 @@ public function testLoadError(): void */ public function testGetClassName(): void { - $this->assertSame('\\' . \GaletteAuto\Finition::class, \GaletteAuto\Finition::getClassForPropName('finition')); + $this->assertSame(\GaletteAuto\Finition::class, \GaletteAuto\AbstractObject::getClassForPropName('finition')); } } diff --git a/tests/GaletteAuto/tests/units/State.php b/tests/GaletteAuto/tests/units/State.php index 93b5447..423735c 100644 --- a/tests/GaletteAuto/tests/units/State.php +++ b/tests/GaletteAuto/tests/units/State.php @@ -98,6 +98,6 @@ public function testLoadError(): void */ public function testGetClassName(): void { - $this->assertSame('\\' . \GaletteAuto\State::class, \GaletteAuto\State::getClassForPropName('state')); + $this->assertSame(\GaletteAuto\State::class, \GaletteAuto\AbstractObject::getClassForPropName('state')); } } diff --git a/tests/GaletteAuto/tests/units/Transmission.php b/tests/GaletteAuto/tests/units/Transmission.php index 8ec67c2..5ac241f 100644 --- a/tests/GaletteAuto/tests/units/Transmission.php +++ b/tests/GaletteAuto/tests/units/Transmission.php @@ -98,6 +98,6 @@ public function testLoadError(): void */ public function testGetClassName(): void { - $this->assertSame('\\' . \GaletteAuto\Transmission::class, \GaletteAuto\Transmission::getClassForPropName('transmission')); + $this->assertSame(\GaletteAuto\Transmission::class, \GaletteAuto\AbstractObject::getClassForPropName('transmission')); } } From 705aa2830bf692a26938ee12dca5de03a9bd5b32 Mon Sep 17 00:00:00 2001 From: Johan Cwiklinski Date: Sat, 26 Sep 2026 18:20:36 +0200 Subject: [PATCH 2/3] Common repository for models and properties, count on a subquery that follows the brand filter --- lib/GaletteAuto/AbstractObject.php | 143 --------------- lib/GaletteAuto/Controllers/Controller.php | 32 +++- .../Controllers/Crud/ModelsController.php | 22 +-- .../Controllers/Crud/PropertiesController.php | 26 ++- lib/GaletteAuto/Model.php | 22 --- .../Repository/AbstractRepository.php | 165 ++++++++++++++++++ lib/GaletteAuto/Repository/Models.php | 145 +++++---------- lib/GaletteAuto/Repository/Properties.php | 98 +++++++++++ templates/default/model.html.twig | 2 +- templates/default/object_list.html.twig | 15 +- templates/default/vehicles.html.twig | 12 +- .../tests/units/ModelsController.php | 2 +- .../tests/units/PropertiesController.php | 2 +- tests/GaletteAuto/tests/units/Body.php | 45 +++-- tests/GaletteAuto/tests/units/Brand.php | 45 +++-- tests/GaletteAuto/tests/units/Color.php | 47 +++-- tests/GaletteAuto/tests/units/Finition.php | 45 +++-- tests/GaletteAuto/tests/units/Model.php | 15 +- tests/GaletteAuto/tests/units/State.php | 45 +++-- .../GaletteAuto/tests/units/Transmission.php | 45 +++-- 20 files changed, 555 insertions(+), 418 deletions(-) create mode 100644 lib/GaletteAuto/Repository/AbstractRepository.php create mode 100644 lib/GaletteAuto/Repository/Properties.php diff --git a/lib/GaletteAuto/AbstractObject.php b/lib/GaletteAuto/AbstractObject.php index 7ce31b1..1b3fc36 100644 --- a/lib/GaletteAuto/AbstractObject.php +++ b/lib/GaletteAuto/AbstractObject.php @@ -12,11 +12,8 @@ use ArrayObject; use Analog\Analog; -use Laminas\Db\Sql\Expression; -use Laminas\Db\Sql\Select; use Slim\Routing\RouteParser; use Galette\Core\Db; -use GaletteAuto\Filters\PropertiesList; /** * Automobile Object abstract class for galette Auto plugin @@ -48,9 +45,6 @@ abstract class AbstractObject protected Db $zdb; protected ?int $id = null; protected ?string $value = null; - protected ?PropertiesList $filters = null; - - private int $count; /** * Default constructor @@ -78,31 +72,6 @@ public static function fromPropertyName(Db $zdb, string $property): self return new $class($zdb); } - /** - * Get the list - * - * @return array> - */ - public function getList(): array - { - try { - $select = $this->buildSelect(); - $results = $this->zdb->execute($select); - $list = []; - foreach ($results as $row) { - $list[] = $row; - } - return $list; - } catch (\Exception $e) { - Analog::log( - '[' . get_class($this) . '] Cannot load ' . static::TABLE - . ' list | ' . $e->getMessage(), - Analog::ERROR - ); - throw $e; - } - } - /** * Loads a record * @@ -189,39 +158,6 @@ public function store(bool $new = false): bool } } - /** - * Delete some records - * - * @param int[] $ids Array of records id to delete - */ - public function delete(array $ids): bool - { - try { - $delete = $this->zdb->delete(AUTO_PREFIX . static::TABLE); - $delete->where->in(static::PK, $ids); - $this->zdb->execute($delete); - return true; - } catch (\Exception $e) { - Analog::log( - '[' . get_class($this) . '] Cannot delete ' . static::TABLE - . ' from ids `' . implode(' - ', $ids) . '` | ' . $e->getMessage(), - Analog::WARNING - ); - throw $e; - } - } - - /** - * Set filters - * - * @param PropertiesList $filters Filters - */ - public function setFilters(PropertiesList $filters): self - { - $this->filters = $filters; - return $this; - } - /** * Get field label */ @@ -344,83 +280,4 @@ public static function getClassForPropName(string $property): string } return self::CLASSES[$property]; } - - /** - * Builds the SELECT statement - * - * @return Select SELECT statement - */ - private function buildSelect(): Select - { - try { - $select = $this->zdb->select(AUTO_PREFIX . static::TABLE); - $select->order([static::FIELD . ' ASC', static::PK . ' ASC']); - if (isset($this->filters)) { - $this->filters->setLimits($select); - } - $this->proceedCount($select); - - return $select; - } catch (\Exception $e) { - Analog::log( - 'Cannot build SELECT clause | ' . $e->getMessage(), - Analog::WARNING - ); - throw $e; - } - } - - /** - * Count objects from the query - * - * @param Select $select Original select - */ - private function proceedCount(Select $select): void - { - try { - $countSelect = clone $select; - $countSelect->reset($countSelect::COLUMNS); - $countSelect->reset($countSelect::JOINS); - $countSelect->reset($countSelect::ORDER); - $countSelect->reset($countSelect::LIMIT); - $countSelect->reset($countSelect::OFFSET); - $countSelect->columns( - [ - static::PK => new Expression('COUNT(' . static::PK . ')') - ] - ); - - $results = $this->zdb->execute($countSelect); - $result = $results->current(); - - $k = static::PK; - $this->count = (int)$result->$k; - - if ($this->count > 0 && isset($this->filters)) { - $this->filters->setCounter($this->count); - } - } catch (\Exception $e) { - Analog::log( - 'Cannot count models | ' . $e->getMessage(), - Analog::WARNING - ); - throw $e; - } - } - - /** - * Get count for list - */ - public function getCount(): int - { - return $this->count; - } - - /** - * Display localized count for object - */ - public function displayCount(): string - { - return $this->getCountLabel($this->getCount()); - } } diff --git a/lib/GaletteAuto/Controllers/Controller.php b/lib/GaletteAuto/Controllers/Controller.php index 6388abc..b233e14 100644 --- a/lib/GaletteAuto/Controllers/Controller.php +++ b/lib/GaletteAuto/Controllers/Controller.php @@ -12,10 +12,17 @@ use Analog\Analog; use Galette\Repository\Members; +use GaletteAuto\AbstractObject; use GaletteAuto\Auto; +use GaletteAuto\Body; +use GaletteAuto\Brand; +use GaletteAuto\Color; +use GaletteAuto\Finition; use GaletteAuto\History; use GaletteAuto\Model; use GaletteAuto\Picture; +use GaletteAuto\State; +use GaletteAuto\Transmission; use GaletteAuto\VehicleAccess; use Laminas\Db\ResultSet\ResultSet; use Slim\Psr7\Request; @@ -25,6 +32,7 @@ use GaletteAuto\Filters\ModelsList; use GaletteAuto\Filters\AutosList; use GaletteAuto\Repository\Models; +use GaletteAuto\Repository\Properties; use GaletteAuto\Repository\Vehicles; use DI\Attribute\Inject; @@ -75,6 +83,18 @@ protected function getVehicles(): Vehicles return new Vehicles($this->plugins, $this->zdb, $this->login, $this->history); } + /** + * Get the whole list of a property + * + * @param class-string $class Property class name + * + * @return array + */ + protected function getProperties(string $class): array + { + return (new Properties($this->zdb, $this->preferences, $this->login, $class))->getList(); + } + /** * Can current user manage all the vehicles? * @@ -369,12 +389,12 @@ public function showAddEditVehicle(Request $request, Response $response, string 'require_dialog' => true, 'car' => $auto, '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(), + 'brands' => $this->getProperties(Brand::class), + 'colors' => $this->getProperties(Color::class), + 'bodies' => $this->getProperties(Body::class), + 'transmissions' => $this->getProperties(Transmission::class), + 'finitions' => $this->getProperties(Finition::class), + 'states' => $this->getProperties(State::class), 'fuels' => $auto->listFuels(), 'time' => time(), 'required' => $auto->getRequired() diff --git a/lib/GaletteAuto/Controllers/Crud/ModelsController.php b/lib/GaletteAuto/Controllers/Crud/ModelsController.php index 721721e..4f2f5d1 100644 --- a/lib/GaletteAuto/Controllers/Crud/ModelsController.php +++ b/lib/GaletteAuto/Controllers/Crud/ModelsController.php @@ -16,6 +16,7 @@ use GaletteAuto\Filters\ModelsList; use GaletteAuto\Model; use GaletteAuto\Repository\Models; +use GaletteAuto\Repository\Properties; use Slim\Psr7\Request; use Slim\Psr7\Response; @@ -192,12 +193,12 @@ public function edit(Request $request, Response $response, ?int $id = null, stri } } - $brand = new Brand($this->zdb); + $brands = new Properties($this->zdb, $this->preferences, $this->login, Brand::class); $params = [ 'page_title' => $title, 'mode' => ($action === 'add' ? 'new' : 'modif'), 'model' => $model, - 'brands' => $brand->getList(), + 'brands' => $brands->getList(), ]; // display page @@ -335,16 +336,17 @@ public function confirmRemoveTitle(array $args): string */ protected function doDelete(array $args, array $post): bool { - $model = new Model($this->zdb); - - if (!is_array($post['id'])) { - $ids = (array)$post['id']; - } else { - $ids = $post['id']; - } + $ids = array_map('intval', (array)$post['id']); + $models = new Models( + $this->zdb, + $this->preferences, + $this->login, + new ModelsList() + ); try { - return $model->delete($ids); + $models->remove($ids); + return true; } catch (\Throwable $e) { if ($this->zdb->isForeignKeyException($e)) { $this->flash->addMessage( diff --git a/lib/GaletteAuto/Controllers/Crud/PropertiesController.php b/lib/GaletteAuto/Controllers/Crud/PropertiesController.php index 24b1a3f..ed095ca 100644 --- a/lib/GaletteAuto/Controllers/Crud/PropertiesController.php +++ b/lib/GaletteAuto/Controllers/Crud/PropertiesController.php @@ -24,6 +24,7 @@ use GaletteAuto\Filters\ModelsList; use GaletteAuto\Filters\PropertiesList; use GaletteAuto\Repository\Models; +use GaletteAuto\Repository\Properties; /** * Galette Auto plugin controller for properties (brands, models, colors, ...) @@ -148,7 +149,6 @@ protected function propertiesList( if (isset($get['nbshow']) && is_numeric($get['nbshow'])) { $filters->show = $get['nbshow']; } - $obj->setFilters($filters); switch ($option) { case 'page': @@ -161,9 +161,11 @@ protected function propertiesList( $this->saveFilters($obj, $filters); + $properties = $this->getRepository($obj::class, $filters); $params = [ 'page_title' => $obj->getListTitle(), - 'list' => $obj->getList(), + 'list' => $properties->getList(), + 'count_label' => $obj->getCountLabel($properties->getCount()), 'field_name' => $obj->getFieldLabel(), 'add_text' => $obj->getAddText(), 'obj' => $obj, @@ -466,16 +468,11 @@ public function doRemoveProperty(Request $request, Response $response, string $p _T("Removal has not been confirmed!") ); } else { - if (!is_array($post['id'])) { - $ids = (array)$post['id']; - } else { - $ids = $post['id']; - } - + $ids = array_map('intval', (array)$post['id']); $object = AbstractObject::fromPropertyName($this->zdb, $property); try { - $object->delete($ids); + $this->getRepository($object::class)->remove($ids); $this->flash->addMessage( 'success_detected', $object->getRemovedMessage(count($ids)) @@ -505,6 +502,17 @@ public function doRemoveProperty(Request $request, Response $response, string $p } } + /** + * Get properties repository + * + * @param class-string $class Property class name + * @param ?PropertiesList $filters Filters + */ + protected function getRepository(string $class, ?PropertiesList $filters = null): Properties + { + return new Properties($this->zdb, $this->preferences, $this->login, $class, $filters); + } + /** * Get filters * diff --git a/lib/GaletteAuto/Model.php b/lib/GaletteAuto/Model.php index 482c4cf..b5f99ac 100644 --- a/lib/GaletteAuto/Model.php +++ b/lib/GaletteAuto/Model.php @@ -144,28 +144,6 @@ public function store(bool $new = false): bool } } - /** - * Delete some models - * - * @param array $ids Array of models id to delete - */ - public function delete(array $ids): bool - { - try { - $delete = $this->zdb->delete(AUTO_PREFIX . self::TABLE); - $delete->where->in(self::PK, $ids); - $this->zdb->execute($delete); - return true; - } catch (\Exception $e) { - Analog::log( - '[' . get_class($this) . '] Cannot delete models from ids `' - . implode(' - ', $ids) . '` | ' . $e->getMessage(), - Analog::WARNING - ); - throw $e; - } - } - /** * Get model ID */ diff --git a/lib/GaletteAuto/Repository/AbstractRepository.php b/lib/GaletteAuto/Repository/AbstractRepository.php new file mode 100644 index 0000000..784ff58 --- /dev/null +++ b/lib/GaletteAuto/Repository/AbstractRepository.php @@ -0,0 +1,165 @@ + + */ +abstract class AbstractRepository extends Repository +{ + /** Table alias used in queries */ + protected const string ALIAS = ''; + + private int $count = 0; + + /** + * Constructor + * + * @param Db $zdb Database instance + * @param Preferences $preferences Preferences instance + * @param Login $login Logged in instance + * @param string $entity Entity class name, relative to the plugin namespace + * @param Pagination $filters Filtering + */ + public function __construct( + Db $zdb, + Preferences $preferences, + Login $login, + string $entity, + Pagination $filters + ) { + parent::__construct($zdb, $preferences, $login, $entity, 'GaletteAuto', AUTO_PREFIX); + $this->filters = $filters; + } + + /** + * Get table name, without prefixes + */ + abstract protected function getTable(): string; + + /** + * Get primary key name + */ + abstract protected function getPk(): string; + + /** + * Builds the SELECT statement, neither ordered nor limited + */ + protected function buildSelect(): Select + { + return $this->zdb->select(AUTO_PREFIX . $this->getTable(), static::ALIAS); + } + + /** + * Builds the order clause + * + * @return array + */ + abstract protected function buildOrderClause(): array; + + /** + * Get the rows, counting all of them + * + * @param Select $select Select, filtered + * @param bool $limit Only retrieve the rows of the current page + */ + protected function fetchRows(Select $select, bool $limit): ResultSet + { + $select->order($this->buildOrderClause()); + $this->proceedCount($select); + if ($limit) { + $this->filters->setLimits($select); + } + + /** @var ResultSet $rows */ + $rows = $this->zdb->execute($select); + return $rows; + } + + /** + * Count rows matching the query + * + * Counting on a subquery keeps every filter right. + * + * @param Select $select Original select + */ + private function proceedCount(Select $select): void + { + $counted = clone $select; + $counted->reset(Select::COLUMNS); + $counted->reset(Select::ORDER); + $counted->reset(Select::JOINS); + $counted->columns(['id' => new Expression(static::ALIAS . '.' . $this->getPk())]); + foreach ($select->joins as $join) { + $counted->join($join['name'], $join['on'], [], $join['type']); + } + + $count_select = new Select(['counted' => $counted]); + $count_select->columns(['count' => new Expression('COUNT(*)')]); + + $result = $this->zdb->execute($count_select)->current(); + $this->count = (int)$result['count']; + $this->filters->setCounter($this->count); + } + + /** + * Get count for last list + */ + public function getCount(): int + { + return $this->count; + } + + /** + * Remove records + * + * @param array $ids Records IDs + * + * @throws \Throwable + */ + public function remove(array $ids): void + { + try { + $delete = $this->zdb->delete(AUTO_PREFIX . $this->getTable()); + $delete->where->in($this->getPk(), $ids); + $this->zdb->execute($delete); + } catch (\Throwable $e) { + Analog::log( + '[' . static::class . '] Cannot remove ' . $this->getTable() . ' #' . implode(', #', $ids) + . ' | ' . $e->getMessage(), + Analog::ERROR + ); + throw $e; + } + } + + /** + * Nothing to initialize + * + * @param bool $check_first Check first if it seems initialized + */ + public function installInit(bool $check_first = true): bool + { + return true; + } +} diff --git a/lib/GaletteAuto/Repository/Models.php b/lib/GaletteAuto/Repository/Models.php index 421efd5..c4dc815 100644 --- a/lib/GaletteAuto/Repository/Models.php +++ b/lib/GaletteAuto/Repository/Models.php @@ -13,13 +13,10 @@ use Galette\Core\Db; use Galette\Core\Preferences; use Galette\Core\Login; -use Galette\Repository\Repository; use GaletteAuto\Model; use GaletteAuto\Brand; use GaletteAuto\Filters\ModelsList; -use Analog\Analog; use Laminas\Db\ResultSet\ResultSet; -use Laminas\Db\Sql\Expression; use Laminas\Db\Sql\Select; /** @@ -28,12 +25,9 @@ * @author Johan Cwiklinski */ -class Models extends Repository +class Models extends AbstractRepository { - public const string TABLE = Model::TABLE; - public const string PK = Model::PK; - - private int $count; + protected const string ALIAS = 'm'; /** * Main constructor @@ -45,69 +39,63 @@ class Models extends Repository */ public function __construct(Db $zdb, Preferences $preferences, Login $login, ModelsList $filters) { - parent::__construct($zdb, $preferences, $login, null, 'GaletteAuto', AUTO_PREFIX); - $this->setFilters($filters); + parent::__construct($zdb, $preferences, $login, 'Model', $filters); } /** * Get the list of all models * - * @param ?int $brandId Optional brand we want models for + * @param ?int $brandId Optional brand we want models for; the whole list is retrieved then * @param bool $as_object Whether to return an array of objects or a ResultSet * - * @return array|ResultSet + * @return ($as_object is true ? array : ResultSet) */ public function getList(?int $brandId = null, bool $as_object = true): array|ResultSet { $select = $this->buildSelect(); - if ($brandId !== null) { - $select->where( - [ - 'm.' . Brand::PK => $brandId - ] - ); - } else { - $this->filters->setLimits($select); + $select->where(['m.' . Brand::PK => $brandId]); } - $results = $this->zdb->execute($select); - - if ($as_object) { - $models = []; - foreach ($results as $r) { - $pk = self::PK; - $models[$r->$pk] = new Model($this->zdb, $r); - } - return $models; - } else { + $results = $this->fetchRows($select, $brandId === null); + + if (!$as_object) { return $results; } + + $models = []; + foreach ($results as $r) { + $models[(int)$r[Model::PK]] = new Model($this->zdb, $r); + } + return $models; + } + + /** + * Get table name, without prefixes + */ + protected function getTable(): string + { + return Model::TABLE; + } + + /** + * Get primary key name + */ + protected function getPk(): string + { + return Model::PK; } /** * Builds the SELECT statement - * - * @return Select SELECT statement */ - private function buildSelect(): Select + protected function buildSelect(): Select { - try { - $select = $this->zdb->select(AUTO_PREFIX . self::TABLE, 'm'); - $select->join( - ['b' => PREFIX_DB . AUTO_PREFIX . Brand::TABLE], - 'm.' . Brand::PK . '= b.' . Brand::PK - ); - $select->order(self::buildOrderClause()); - $this->proceedCount($select); - - return $select; - } catch (\Exception $e) { - Analog::log( - 'Cannot build SELECT clause for models | ' . $e->getMessage(), - Analog::WARNING - ); - throw $e; - } + $select = parent::buildSelect(); + $select->join( + ['b' => PREFIX_DB . AUTO_PREFIX . Brand::TABLE], + 'm.' . Brand::PK . ' = b.' . Brand::PK + ); + return $select; } /** @@ -115,7 +103,7 @@ private function buildSelect(): Select * * @return array SQL ORDER clause */ - private function buildOrderClause(): array + protected function buildOrderClause(): array { $order = []; @@ -128,61 +116,8 @@ private function buildOrderClause(): array $order[] = 'm.model ' . $this->filters->getDirection(); break; } + $order[] = 'm.' . Model::PK . ' ASC'; return $order; } - - /** - * Count contributions from the query - * - * @param Select $select Original select - */ - private function proceedCount(Select $select): void - { - try { - $countSelect = clone $select; - $countSelect->reset($countSelect::COLUMNS); - $countSelect->reset($countSelect::JOINS); - $countSelect->reset($countSelect::ORDER); - $countSelect->columns( - [ - self::PK => new Expression('COUNT(' . self::PK . ')') - ] - ); - - $results = $this->zdb->execute($countSelect); - $result = $results->current(); - - $k = self::PK; - $this->count = (int)$result->$k; - - if ($this->count > 0) { - $this->filters->setCounter($this->count); - } - } catch (\Exception $e) { - Analog::log( - 'Cannot count models | ' . $e->getMessage(), - Analog::WARNING - ); - throw $e; - } - } - - /** - * Add default values in database - * - * @param bool $check_first Check first if it seems initialized, defaults to true - */ - public function installInit(bool $check_first = true): bool - { - return true; - } - - /** - * Get count for current query - */ - public function getCount(): int - { - return $this->count; - } } diff --git a/lib/GaletteAuto/Repository/Properties.php b/lib/GaletteAuto/Repository/Properties.php new file mode 100644 index 0000000..1656662 --- /dev/null +++ b/lib/GaletteAuto/Repository/Properties.php @@ -0,0 +1,98 @@ + + */ +class Properties extends AbstractRepository +{ + protected const string ALIAS = 'p'; + + private bool $paginate; + + /** + * Constructor + * + * @param Db $zdb Database instance + * @param Preferences $preferences Preferences instance + * @param Login $login Logged in instance + * @param class-string $class Property class name + * @param ?PropertiesList $filters Filters; the whole list is retrieved without them + */ + public function __construct( + Db $zdb, + Preferences $preferences, + Login $login, + private string $class, + ?PropertiesList $filters = null + ) { + $this->paginate = $filters !== null; + parent::__construct( + $zdb, + $preferences, + $login, + substr($class, strrpos($class, '\\') + 1), + $filters ?? new PropertiesList() + ); + } + + /** + * Get the list, sorted by value + * + * @return array + */ + public function getList(): array + { + $list = []; + foreach ($this->fetchRows($this->buildSelect(), $this->paginate) as $row) { + $list[] = (new $this->class($this->zdb))->loadFromRow($row); + } + return $list; + } + + /** + * Get table name, without prefixes + */ + protected function getTable(): string + { + return $this->class::TABLE; + } + + /** + * Get primary key name + */ + protected function getPk(): string + { + return $this->class::PK; + } + + /** + * Builds the order clause + * + * @return array + */ + protected function buildOrderClause(): array + { + return [ + self::ALIAS . '.' . $this->class::FIELD . ' ASC', + self::ALIAS . '.' . $this->class::PK . ' ASC' + ]; + } +} diff --git a/templates/default/model.html.twig b/templates/default/model.html.twig index 04424dd..f94ea01 100644 --- a/templates/default/model.html.twig +++ b/templates/default/model.html.twig @@ -24,7 +24,7 @@ {% set brand_list_values = {(-1): _T("Choose a brand", "auto")} %} {% for brand in brands %} - {% set brand_list_values = brand_list_values + {(brand.id_brand): brand.brand} %} + {% set brand_list_values = brand_list_values + {(brand.getId()): brand.getValue()} %} {% endfor %} {% if brands|length > 0 %} diff --git a/templates/default/object_list.html.twig b/templates/default/object_list.html.twig index a3ce25d..8b15a77 100644 --- a/templates/default/object_list.html.twig +++ b/templates/default/object_list.html.twig @@ -6,14 +6,13 @@ {% extends 'elements/list.html.twig' %} -{% set pk = obj.getPk() %} {% set field = obj.getField() %} {% set nb = list|length %} {% block infoline %} {% set infoline = { - 'label': obj.displayCount(), + 'label': count_label, 'route': { 'name': 'propertyFilter', 'args': {'property': field} @@ -41,26 +40,26 @@ {% block body %} {% for o in list %} - {% set edit_link = url_for("propertyEdit", {"property": obj.getRouteName(), "id": o[pk]}) %} + {% set edit_link = url_for("propertyEdit", {"property": obj.getRouteName(), "id": o.getId()}) %} - {{ o[field] }} + {{ o.getValue() }} - + - + diff --git a/templates/default/vehicles.html.twig b/templates/default/vehicles.html.twig index 8ca5a9b..0b62afa 100644 --- a/templates/default/vehicles.html.twig +++ b/templates/default/vehicles.html.twig @@ -28,7 +28,7 @@ {% set brand_list_values = {(-1): _T("Choose a brand", "auto")} %} {% for brand in brands %} - {% set brand_list_values = brand_list_values + {(brand.id_brand): brand.brand} %} + {% set brand_list_values = brand_list_values + {(brand.getId()): brand.getValue()} %} {% endfor %} {% include "components/forms/select.html.twig" with { @@ -157,7 +157,7 @@ {% set color_list_values = {(-1): _T("Choose a color", "auto")} %} {% for color in colors %} - {% set color_list_values = color_list_values + {(color.id_color): color.color} %} + {% set color_list_values = color_list_values + {(color.getId()): color.getValue()} %} {% endfor %} {% include "components/forms/select.html.twig" with { @@ -170,7 +170,7 @@ {% set state_list_values = {(-1): _T("Choose a state", "auto")} %} {% for state in states %} - {% set state_list_values = state_list_values + {(state.id_state): state.state} %} + {% set state_list_values = state_list_values + {(state.getId()): state.getValue()} %} {% endfor %} {% include "components/forms/select.html.twig" with { @@ -199,7 +199,7 @@
{% set body_list_values = {(-1): _T("Choose a body", "auto")} %} {% for body in bodies %} - {% set body_list_values = body_list_values + {(body.id_body): body.body} %} + {% set body_list_values = body_list_values + {(body.getId()): body.getValue()} %} {% endfor %} {% include "components/forms/select.html.twig" with { @@ -212,7 +212,7 @@ {% set transmission_list_values = {(-1): _T("Choose a transmission", "auto")} %} {% for transmission in transmissions %} - {% set transmission_list_values = transmission_list_values + {(transmission.id_transmission): transmission.transmission} %} + {% set transmission_list_values = transmission_list_values + {(transmission.getId()): transmission.getValue()} %} {% endfor %} {% include "components/forms/select.html.twig" with { @@ -225,7 +225,7 @@ {% set finition_list_values = {(-1): _T("Choose a finition", "auto")} %} {% for finition in finitions %} - {% set finition_list_values = finition_list_values + {(finition.id_finition): finition.finition} %} + {% set finition_list_values = finition_list_values + {(finition.getId()): finition.getValue()} %} {% endfor %} {% include "components/forms/select.html.twig" with { diff --git a/tests/GaletteAuto/Controllers/tests/units/ModelsController.php b/tests/GaletteAuto/Controllers/tests/units/ModelsController.php index b1a9ed6..d78bf07 100644 --- a/tests/GaletteAuto/Controllers/tests/units/ModelsController.php +++ b/tests/GaletteAuto/Controllers/tests/units/ModelsController.php @@ -234,7 +234,7 @@ public function testRemove(): void $this->app->handle($request); $this->expectFlashData(['error_detected' => ['This model is used by one or more vehicles, it cannot be deleted.']]); $this->expectLogEntry(\Analog\Analog::ERROR, 'Query error: DELETE FROM'); - $this->expectLogEntry(\Analog\Analog::WARNING, 'Cannot delete models from ids `' . $used . '`'); + $this->expectLogEntry(\Analog\Analog::ERROR, 'Cannot remove models #' . $used . ' |'); $this->expectNoLogEntry(); if (!$this->zdb->isPostgres()) { $this->expected_mysql_warnings[] = new \ArrayObject([ diff --git a/tests/GaletteAuto/Controllers/tests/units/PropertiesController.php b/tests/GaletteAuto/Controllers/tests/units/PropertiesController.php index 29bf254..6d898b2 100644 --- a/tests/GaletteAuto/Controllers/tests/units/PropertiesController.php +++ b/tests/GaletteAuto/Controllers/tests/units/PropertiesController.php @@ -274,7 +274,7 @@ public function testRemove(): void ['error_detected' => ['This color is used by one or more vehicles, it cannot be deleted.']] ); $this->expectLogEntry(\Analog\Analog::ERROR, 'Query error: DELETE FROM'); - $this->expectLogEntry(\Analog\Analog::WARNING, 'Cannot delete colors from ids `' . $used . '`'); + $this->expectLogEntry(\Analog\Analog::ERROR, 'Cannot remove colors #' . $used . ' |'); $this->expectNoLogEntry(); if (!$this->zdb->isPostgres()) { $this->expected_mysql_warnings[] = new \ArrayObject([ diff --git a/tests/GaletteAuto/tests/units/Body.php b/tests/GaletteAuto/tests/units/Body.php index f4906e9..b3c5265 100644 --- a/tests/GaletteAuto/tests/units/Body.php +++ b/tests/GaletteAuto/tests/units/Body.php @@ -27,10 +27,16 @@ class Body extends GaletteTestCase public function testEmpty(): void { $body = new \GaletteAuto\Body($this->zdb); + $bodies = new \GaletteAuto\Repository\Properties( + $this->zdb, + $this->preferences, + $this->login, + \GaletteAuto\Body::class + ); $this->assertSame('Body', $body->getFieldLabel()); - $this->assertCount(0, $body->getList()); - $this->assertSame('0 bodies', $body->displayCount()); + $this->assertCount(0, $bodies->getList()); + $this->assertSame('0 bodies', $body->getCountLabel($bodies->getCount())); } /** @@ -39,20 +45,26 @@ public function testEmpty(): void public function testCrud(): void { $body = new \GaletteAuto\Body($this->zdb); + $bodies = new \GaletteAuto\Repository\Properties( + $this->zdb, + $this->preferences, + $this->login, + \GaletteAuto\Body::class + ); //ensure the table is empty - $this->assertCount(0, $body->getList()); + $this->assertCount(0, $bodies->getList()); //Add new body $body->setValue('Coupe'); $this->assertTrue($body->store(true)); $first_id = $body->getId(); - $this->assertCount(1, $body->getList()); - $listed_body = $body->getList()[0]; - $this->assertInstanceOf(\ArrayObject::class, $listed_body); - $this->assertGreaterThan(0, $listed_body['id_body']); - $this->assertSame('Coupe', $listed_body['body']); - $this->assertSame('1 body', $body->displayCount()); + $this->assertCount(1, $bodies->getList()); + $listed_body = $bodies->getList()[0]; + $this->assertInstanceOf(\GaletteAuto\Body::class, $listed_body); + $this->assertGreaterThan(0, $listed_body->getId()); + $this->assertSame('Coupe', $listed_body->getValue()); + $this->assertSame('1 body', $body->getCountLabel($bodies->getCount())); //add another one $body = new \GaletteAuto\Body($this->zdb); @@ -60,23 +72,22 @@ public function testCrud(): void $this->assertTrue($body->store(true)); $id = $body->getId(); - $this->assertCount(2, $body->getList()); - $this->assertSame('2 bodies', $body->displayCount()); + $this->assertCount(2, $bodies->getList()); + $this->assertSame('2 bodies', $body->getCountLabel($bodies->getCount())); $body = new \GaletteAuto\Body($this->zdb); $this->assertTrue($body->load($id)); $body->setValue('Break'); $this->assertTrue($body->store()); - $this->assertCount(2, $body->getList()); - $this->assertSame('2 bodies', $body->displayCount()); + $this->assertCount(2, $bodies->getList()); + $this->assertSame('2 bodies', $body->getCountLabel($bodies->getCount())); - $body = new \GaletteAuto\Body($this->zdb); - $this->assertTrue($body->delete([$first_id])); - $list = $body->getList(); + $bodies->remove([$first_id]); + $list = $bodies->getList(); $this->assertCount(1, $list); $last_body = $list[0]; - $this->assertSame($id, (int)$last_body['id_body']); + $this->assertSame($id, $last_body->getId()); } /** diff --git a/tests/GaletteAuto/tests/units/Brand.php b/tests/GaletteAuto/tests/units/Brand.php index 8a51cf9..047950e 100644 --- a/tests/GaletteAuto/tests/units/Brand.php +++ b/tests/GaletteAuto/tests/units/Brand.php @@ -27,10 +27,16 @@ class Brand extends GaletteTestCase public function testEmpty(): void { $brand = new \GaletteAuto\Brand($this->zdb); + $brands = new \GaletteAuto\Repository\Properties( + $this->zdb, + $this->preferences, + $this->login, + \GaletteAuto\Brand::class + ); $this->assertSame('Brand', $brand->getFieldLabel()); - $this->assertCount(0, $brand->getList()); - $this->assertSame('0 brands', $brand->displayCount()); + $this->assertCount(0, $brands->getList()); + $this->assertSame('0 brands', $brand->getCountLabel($brands->getCount())); } /** @@ -39,20 +45,26 @@ public function testEmpty(): void public function testCrud(): void { $brand = new \GaletteAuto\Brand($this->zdb); + $brands = new \GaletteAuto\Repository\Properties( + $this->zdb, + $this->preferences, + $this->login, + \GaletteAuto\Brand::class + ); //ensure the table is empty - $this->assertCount(0, $brand->getList()); + $this->assertCount(0, $brands->getList()); //Add new brand $brand->setValue('Audi'); $this->assertTrue($brand->store(true)); $first_id = $brand->getId(); - $this->assertCount(1, $brand->getList()); - $listed_brand = $brand->getList()[0]; - $this->assertInstanceOf(\ArrayObject::class, $listed_brand); - $this->assertGreaterThan(0, $listed_brand['id_brand']); - $this->assertSame('Audi', $listed_brand['brand']); - $this->assertSame('1 brand', $brand->displayCount()); + $this->assertCount(1, $brands->getList()); + $listed_brand = $brands->getList()[0]; + $this->assertInstanceOf(\GaletteAuto\Brand::class, $listed_brand); + $this->assertGreaterThan(0, $listed_brand->getId()); + $this->assertSame('Audi', $listed_brand->getValue()); + $this->assertSame('1 brand', $brand->getCountLabel($brands->getCount())); //add another one $brand = new \GaletteAuto\Brand($this->zdb); @@ -60,23 +72,22 @@ public function testCrud(): void $this->assertTrue($brand->store(true)); $id = $brand->getId(); - $this->assertCount(2, $brand->getList()); - $this->assertSame('2 brands', $brand->displayCount()); + $this->assertCount(2, $brands->getList()); + $this->assertSame('2 brands', $brand->getCountLabel($brands->getCount())); $brand = new \GaletteAuto\Brand($this->zdb); $this->assertTrue($brand->load($id)); $brand->setValue('Mercedes'); $this->assertTrue($brand->store()); - $this->assertCount(2, $brand->getList()); - $this->assertSame('2 brands', $brand->displayCount()); + $this->assertCount(2, $brands->getList()); + $this->assertSame('2 brands', $brand->getCountLabel($brands->getCount())); - $brand = new \GaletteAuto\Brand($this->zdb); - $this->assertTrue($brand->delete([$first_id])); - $list = $brand->getList(); + $brands->remove([$first_id]); + $list = $brands->getList(); $this->assertCount(1, $list); $last_brand = $list[0]; - $this->assertSame($id, (int)$last_brand['id_brand']); + $this->assertSame($id, $last_brand->getId()); } /** diff --git a/tests/GaletteAuto/tests/units/Color.php b/tests/GaletteAuto/tests/units/Color.php index f4a0572..18b0904 100644 --- a/tests/GaletteAuto/tests/units/Color.php +++ b/tests/GaletteAuto/tests/units/Color.php @@ -27,10 +27,16 @@ class Color extends GaletteTestCase public function testEmpty(): void { $color = new \GaletteAuto\Color($this->zdb); + $colors = new \GaletteAuto\Repository\Properties( + $this->zdb, + $this->preferences, + $this->login, + \GaletteAuto\Color::class + ); $this->assertSame('Color', $color->getFieldLabel()); - $this->assertCount(0, $color->getList()); - $this->assertSame('0 colors', $color->displayCount()); + $this->assertCount(0, $colors->getList()); + $this->assertSame('0 colors', $color->getCountLabel($colors->getCount())); } /** @@ -39,20 +45,26 @@ public function testEmpty(): void public function testCrud(): void { $color = new \GaletteAuto\Color($this->zdb); + $colors = new \GaletteAuto\Repository\Properties( + $this->zdb, + $this->preferences, + $this->login, + \GaletteAuto\Color::class + ); //ensure the table is empty - $this->assertCount(0, $color->getList()); + $this->assertCount(0, $colors->getList()); //Add new color $color->setValue('Red'); $this->assertTrue($color->store(true)); $first_id = $color->getId(); - $this->assertCount(1, $color->getList()); - $listed_color = $color->getList()[0]; - $this->assertInstanceOf(\ArrayObject::class, $listed_color); - $this->assertGreaterThan(0, $listed_color['id_color']); - $this->assertSame('Red', $listed_color['color']); - $this->assertSame('1 color', $color->displayCount()); + $this->assertCount(1, $colors->getList()); + $listed_color = $colors->getList()[0]; + $this->assertInstanceOf(\GaletteAuto\Color::class, $listed_color); + $this->assertGreaterThan(0, $listed_color->getId()); + $this->assertSame('Red', $listed_color->getValue()); + $this->assertSame('1 color', $color->getCountLabel($colors->getCount())); //add another one $color = new \GaletteAuto\Color($this->zdb); @@ -60,25 +72,24 @@ public function testCrud(): void $this->assertTrue($color->store(true)); $id = $color->getId(); - $this->assertCount(2, $color->getList()); - $this->assertSame('2 colors', $color->displayCount()); + $this->assertCount(2, $colors->getList()); + $this->assertSame('2 colors', $color->getCountLabel($colors->getCount())); //sorted by value - $this->assertSame(['Blu', 'Red'], array_map(fn($row) => $row['color'], $color->getList())); + $this->assertSame(['Blu', 'Red'], array_map(fn($row) => $row->getValue(), $colors->getList())); $color = new \GaletteAuto\Color($this->zdb); $this->assertTrue($color->load($id)); $color->setValue('Blue'); $this->assertTrue($color->store()); - $this->assertCount(2, $color->getList()); - $this->assertSame('2 colors', $color->displayCount()); + $this->assertCount(2, $colors->getList()); + $this->assertSame('2 colors', $color->getCountLabel($colors->getCount())); - $color = new \GaletteAuto\Color($this->zdb); - $this->assertTrue($color->delete([$first_id])); - $list = $color->getList(); + $colors->remove([$first_id]); + $list = $colors->getList(); $this->assertCount(1, $list); $last_color = $list[0]; - $this->assertSame($id, (int)$last_color['id_color']); + $this->assertSame($id, $last_color->getId()); } /** diff --git a/tests/GaletteAuto/tests/units/Finition.php b/tests/GaletteAuto/tests/units/Finition.php index 79c5d5a..e6308b8 100644 --- a/tests/GaletteAuto/tests/units/Finition.php +++ b/tests/GaletteAuto/tests/units/Finition.php @@ -27,10 +27,16 @@ class Finition extends GaletteTestCase public function testEmpty(): void { $finition = new \GaletteAuto\Finition($this->zdb); + $finitions = new \GaletteAuto\Repository\Properties( + $this->zdb, + $this->preferences, + $this->login, + \GaletteAuto\Finition::class + ); $this->assertSame('Finition', $finition->getFieldLabel()); - $this->assertCount(0, $finition->getList()); - $this->assertSame('0 finitions', $finition->displayCount()); + $this->assertCount(0, $finitions->getList()); + $this->assertSame('0 finitions', $finition->getCountLabel($finitions->getCount())); } /** @@ -39,20 +45,26 @@ public function testEmpty(): void public function testCrud(): void { $finition = new \GaletteAuto\Finition($this->zdb); + $finitions = new \GaletteAuto\Repository\Properties( + $this->zdb, + $this->preferences, + $this->login, + \GaletteAuto\Finition::class + ); //ensure the table is empty - $this->assertCount(0, $finition->getList()); + $this->assertCount(0, $finitions->getList()); //Add new finition $finition->setValue('Feline'); $this->assertTrue($finition->store(true)); $first_id = $finition->getId(); - $this->assertCount(1, $finition->getList()); - $listed_finition = $finition->getList()[0]; - $this->assertInstanceOf(\ArrayObject::class, $listed_finition); - $this->assertGreaterThan(0, $listed_finition['id_finition']); - $this->assertSame('Feline', $listed_finition['finition']); - $this->assertSame('1 finition', $finition->displayCount()); + $this->assertCount(1, $finitions->getList()); + $listed_finition = $finitions->getList()[0]; + $this->assertInstanceOf(\GaletteAuto\Finition::class, $listed_finition); + $this->assertGreaterThan(0, $listed_finition->getId()); + $this->assertSame('Feline', $listed_finition->getValue()); + $this->assertSame('1 finition', $finition->getCountLabel($finitions->getCount())); //add another one $finition = new \GaletteAuto\Finition($this->zdb); @@ -60,23 +72,22 @@ public function testCrud(): void $this->assertTrue($finition->store(true)); $id = $finition->getId(); - $this->assertCount(2, $finition->getList()); - $this->assertSame('2 finitions', $finition->displayCount()); + $this->assertCount(2, $finitions->getList()); + $this->assertSame('2 finitions', $finition->getCountLabel($finitions->getCount())); $finition = new \GaletteAuto\Finition($this->zdb); $this->assertTrue($finition->load($id)); $finition->setValue('RS'); $this->assertTrue($finition->store()); - $this->assertCount(2, $finition->getList()); - $this->assertSame('2 finitions', $finition->displayCount()); + $this->assertCount(2, $finitions->getList()); + $this->assertSame('2 finitions', $finition->getCountLabel($finitions->getCount())); - $finition = new \GaletteAuto\Finition($this->zdb); - $this->assertTrue($finition->delete([$first_id])); - $list = $finition->getList(); + $finitions->remove([$first_id]); + $list = $finitions->getList(); $this->assertCount(1, $list); $last_finition = $list[0]; - $this->assertSame($id, (int)$last_finition['id_finition']); + $this->assertSame($id, $last_finition->getId()); } /** diff --git a/tests/GaletteAuto/tests/units/Model.php b/tests/GaletteAuto/tests/units/Model.php index d43d079..5d1ba3c 100644 --- a/tests/GaletteAuto/tests/units/Model.php +++ b/tests/GaletteAuto/tests/units/Model.php @@ -38,7 +38,13 @@ public function testCrud(): void $this->assertTrue($brand->store(true)); $second_brand_id = $brand->getId(); - $this->assertCount(2, $brand->getList()); + $brands = new \GaletteAuto\Repository\Properties( + $this->zdb, + $this->preferences, + $this->login, + \GaletteAuto\Brand::class + ); + $this->assertCount(2, $brands->getList()); $models = new \GaletteAuto\Repository\Models( $this->zdb, @@ -123,11 +129,14 @@ public function testCrud(): void $this->assertTrue($model->store(true)); $this->assertCount(3, $models->getList()); + $this->assertSame(3, $models->getCount()); $this->assertCount(2, $models->getList($first_brand_id)); + //count follows the brand filter + $this->assertSame(2, $models->getCount()); $this->assertCount(1, $models->getList($second_brand_id)); + $this->assertSame(1, $models->getCount()); - $model = new \GaletteAuto\Model($this->zdb); - $this->assertTrue($model->delete([$id_model])); + $models->remove([$id_model]); $this->assertCount(2, $models->getList()); $this->assertCount(1, $models->getList($first_brand_id)); diff --git a/tests/GaletteAuto/tests/units/State.php b/tests/GaletteAuto/tests/units/State.php index 423735c..000aff1 100644 --- a/tests/GaletteAuto/tests/units/State.php +++ b/tests/GaletteAuto/tests/units/State.php @@ -27,10 +27,16 @@ class State extends GaletteTestCase public function testEmpty(): void { $state = new \GaletteAuto\State($this->zdb); + $states = new \GaletteAuto\Repository\Properties( + $this->zdb, + $this->preferences, + $this->login, + \GaletteAuto\State::class + ); $this->assertSame('State', $state->getFieldLabel()); - $this->assertCount(0, $state->getList()); - $this->assertSame('0 states', $state->displayCount()); + $this->assertCount(0, $states->getList()); + $this->assertSame('0 states', $state->getCountLabel($states->getCount())); } /** @@ -39,20 +45,26 @@ public function testEmpty(): void public function testCrud(): void { $state = new \GaletteAuto\State($this->zdb); + $states = new \GaletteAuto\Repository\Properties( + $this->zdb, + $this->preferences, + $this->login, + \GaletteAuto\State::class + ); //ensure the table is empty - $this->assertCount(0, $state->getList()); + $this->assertCount(0, $states->getList()); //Add new state $state->setValue('Good'); $this->assertTrue($state->store(true)); $first_id = $state->getId(); - $this->assertCount(1, $state->getList()); - $listed_state = $state->getList()[0]; - $this->assertInstanceOf(\ArrayObject::class, $listed_state); - $this->assertGreaterThan(0, $listed_state['id_state']); - $this->assertSame('Good', $listed_state['state']); - $this->assertSame('1 state', $state->displayCount()); + $this->assertCount(1, $states->getList()); + $listed_state = $states->getList()[0]; + $this->assertInstanceOf(\GaletteAuto\State::class, $listed_state); + $this->assertGreaterThan(0, $listed_state->getId()); + $this->assertSame('Good', $listed_state->getValue()); + $this->assertSame('1 state', $state->getCountLabel($states->getCount())); //add another one $state = new \GaletteAuto\State($this->zdb); @@ -60,23 +72,22 @@ public function testCrud(): void $this->assertTrue($state->store(true)); $id = $state->getId(); - $this->assertCount(2, $state->getList()); - $this->assertSame('2 states', $state->displayCount()); + $this->assertCount(2, $states->getList()); + $this->assertSame('2 states', $state->getCountLabel($states->getCount())); $state = new \GaletteAuto\State($this->zdb); $this->assertTrue($state->load($id)); $state->setValue('Wreck'); $this->assertTrue($state->store()); - $this->assertCount(2, $state->getList()); - $this->assertSame('2 states', $state->displayCount()); + $this->assertCount(2, $states->getList()); + $this->assertSame('2 states', $state->getCountLabel($states->getCount())); - $state = new \GaletteAuto\State($this->zdb); - $this->assertTrue($state->delete([$first_id])); - $list = $state->getList(); + $states->remove([$first_id]); + $list = $states->getList(); $this->assertCount(1, $list); $last_state = $list[0]; - $this->assertSame($id, (int)$last_state['id_state']); + $this->assertSame($id, $last_state->getId()); } /** diff --git a/tests/GaletteAuto/tests/units/Transmission.php b/tests/GaletteAuto/tests/units/Transmission.php index 5ac241f..dfe8a3c 100644 --- a/tests/GaletteAuto/tests/units/Transmission.php +++ b/tests/GaletteAuto/tests/units/Transmission.php @@ -27,10 +27,16 @@ class Transmission extends GaletteTestCase public function testEmpty(): void { $transmission = new \GaletteAuto\Transmission($this->zdb); + $transmissions = new \GaletteAuto\Repository\Properties( + $this->zdb, + $this->preferences, + $this->login, + \GaletteAuto\Transmission::class + ); $this->assertSame('Transmission', $transmission->getFieldLabel()); - $this->assertCount(0, $transmission->getList()); - $this->assertSame('0 transmissions', $transmission->displayCount()); + $this->assertCount(0, $transmissions->getList()); + $this->assertSame('0 transmissions', $transmission->getCountLabel($transmissions->getCount())); } /** @@ -39,20 +45,26 @@ public function testEmpty(): void public function testCrud(): void { $transmission = new \GaletteAuto\Transmission($this->zdb); + $transmissions = new \GaletteAuto\Repository\Properties( + $this->zdb, + $this->preferences, + $this->login, + \GaletteAuto\Transmission::class + ); //ensure the table is empty - $this->assertCount(0, $transmission->getList()); + $this->assertCount(0, $transmissions->getList()); //Add new transmission $transmission->setValue('Manual'); $this->assertTrue($transmission->store(true)); $first_id = $transmission->getId(); - $this->assertCount(1, $transmission->getList()); - $listed_transmission = $transmission->getList()[0]; - $this->assertInstanceOf(\ArrayObject::class, $listed_transmission); - $this->assertGreaterThan(0, $listed_transmission['id_transmission']); - $this->assertSame('Manual', $listed_transmission['transmission']); - $this->assertSame('1 transmission', $transmission->displayCount()); + $this->assertCount(1, $transmissions->getList()); + $listed_transmission = $transmissions->getList()[0]; + $this->assertInstanceOf(\GaletteAuto\Transmission::class, $listed_transmission); + $this->assertGreaterThan(0, $listed_transmission->getId()); + $this->assertSame('Manual', $listed_transmission->getValue()); + $this->assertSame('1 transmission', $transmission->getCountLabel($transmissions->getCount())); //add another one $transmission = new \GaletteAuto\Transmission($this->zdb); @@ -60,23 +72,22 @@ public function testCrud(): void $this->assertTrue($transmission->store(true)); $id = $transmission->getId(); - $this->assertCount(2, $transmission->getList()); - $this->assertSame('2 transmissions', $transmission->displayCount()); + $this->assertCount(2, $transmissions->getList()); + $this->assertSame('2 transmissions', $transmission->getCountLabel($transmissions->getCount())); $transmission = new \GaletteAuto\Transmission($this->zdb); $this->assertTrue($transmission->load($id)); $transmission->setValue('Automatic'); $this->assertTrue($transmission->store()); - $this->assertCount(2, $transmission->getList()); - $this->assertSame('2 transmissions', $transmission->displayCount()); + $this->assertCount(2, $transmissions->getList()); + $this->assertSame('2 transmissions', $transmission->getCountLabel($transmissions->getCount())); - $transmission = new \GaletteAuto\Transmission($this->zdb); - $this->assertTrue($transmission->delete([$first_id])); - $list = $transmission->getList(); + $transmissions->remove([$first_id]); + $list = $transmissions->getList(); $this->assertCount(1, $list); $last_transmission = $list[0]; - $this->assertSame($id, (int)$last_transmission['id_transmission']); + $this->assertSame($id, $last_transmission->getId()); } /** From e7047355a72e5dc0ec4f47eefc4c86ffd77780a8 Mon Sep 17 00:00:00 2001 From: Johan Cwiklinski Date: Sat, 26 Sep 2026 18:26:16 +0200 Subject: [PATCH 3/3] Throw on storage errors, answer unknown properties and models with a 404, log errors in one format --- lib/GaletteAuto/AbstractObject.php | 21 ++++---- lib/GaletteAuto/Auto.php | 3 +- .../Controllers/Crud/ModelsController.php | 32 +++++++++--- .../Controllers/Crud/PropertiesController.php | 50 +++++++++++++------ lib/GaletteAuto/History.php | 18 +++---- lib/GaletteAuto/Model.php | 21 ++++---- lib/GaletteAuto/Repository/Vehicles.php | 4 +- .../Controllers/tests/units/Controller.php | 10 ++-- .../tests/units/ModelsController.php | 28 +++++++++-- .../tests/units/PropertiesController.php | 36 ++++++++++--- tests/GaletteAuto/tests/units/Auto.php | 20 ++++---- tests/GaletteAuto/tests/units/Body.php | 8 +-- tests/GaletteAuto/tests/units/Brand.php | 8 +-- tests/GaletteAuto/tests/units/Color.php | 8 +-- tests/GaletteAuto/tests/units/Finition.php | 8 +-- tests/GaletteAuto/tests/units/Model.php | 14 +++--- tests/GaletteAuto/tests/units/State.php | 8 +-- .../GaletteAuto/tests/units/Transmission.php | 8 +-- 18 files changed, 189 insertions(+), 116 deletions(-) diff --git a/lib/GaletteAuto/AbstractObject.php b/lib/GaletteAuto/AbstractObject.php index 1b3fc36..9ce3859 100644 --- a/lib/GaletteAuto/AbstractObject.php +++ b/lib/GaletteAuto/AbstractObject.php @@ -94,10 +94,9 @@ public function load(int $id): bool $this->loadFromRow($result); return true; - } catch (\Exception $e) { + } catch (\Throwable $e) { Analog::log( - '[' . get_class($this) . '] Cannot load ' . static::TABLE - . ' from id `' . $id . '` | ' . $e->getMessage(), + '[' . static::class . '] Cannot load ' . static::FIELD . ' #' . $id . ' | ' . $e->getMessage(), Analog::ERROR ); return false; @@ -120,8 +119,10 @@ public function loadFromRow(ArrayObject $row): self * Store current record * * @param bool $new New record or existing one + * + * @throws \Throwable */ - public function store(bool $new = false): bool + public function store(bool $new = false): void { try { $values = [ @@ -146,15 +147,13 @@ public function store(bool $new = false): bool ); $this->zdb->execute($update); } - return true; - } catch (\Exception $e) { + } catch (\Throwable $e) { Analog::log( - '[' . get_class($this) . '] Cannot store ' . static::TABLE - . ' values `' . ($this->id ?? '') . '`, `' . $this->value . '` | ' - . $e->getMessage(), - Analog::WARNING + '[' . static::class . '] Cannot ' . ($new ? 'add ' : 'update ') . static::FIELD + . ' #' . ($this->id ?? '') . ' | ' . $e->getMessage(), + Analog::ERROR ); - return false; + throw $e; } } diff --git a/lib/GaletteAuto/Auto.php b/lib/GaletteAuto/Auto.php index e732ed3..5b781d0 100644 --- a/lib/GaletteAuto/Auto.php +++ b/lib/GaletteAuto/Auto.php @@ -176,8 +176,7 @@ public function load(int $id): bool return true; } catch (\Exception $e) { Analog::log( - '[' . get_class($this) . '] Cannot load car from id `' . $id - . '` | ' . $e->getMessage(), + '[' . static::class . '] Cannot load vehicle #' . $id . ' | ' . $e->getMessage(), Analog::ERROR ); return false; diff --git a/lib/GaletteAuto/Controllers/Crud/ModelsController.php b/lib/GaletteAuto/Controllers/Crud/ModelsController.php index 4f2f5d1..2943e78 100644 --- a/lib/GaletteAuto/Controllers/Crud/ModelsController.php +++ b/lib/GaletteAuto/Controllers/Crud/ModelsController.php @@ -17,6 +17,7 @@ use GaletteAuto\Model; use GaletteAuto\Repository\Models; use GaletteAuto\Repository\Properties; +use Slim\Exception\HttpNotFoundException; use Slim\Psr7\Request; use Slim\Psr7\Response; @@ -168,8 +169,7 @@ public function edit(Request $request, Response $response, ?int $id = null, stri if ($action === 'edit') { // initialize model structure with database values if (!$model->load((int)$id)) { - //not possible to load, exit - throw new \RuntimeException('Model does not exists!'); + throw new HttpNotFoundException($request); } } @@ -227,7 +227,7 @@ public function doEdit(Request $request, Response $response, ?int $id = null, st $error_detected = []; if (!$is_new && !$model->load((int)$id)) { - throw new \RuntimeException('Model does not exists!'); + throw new HttpNotFoundException($request); } if (!$model->check($post)) { @@ -235,17 +235,17 @@ public function doEdit(Request $request, Response $response, ?int $id = null, st } if (count($error_detected) === 0) { - $res = $model->store($is_new); - if (!$res) { - $error_detected[] - = _T("- An error occurred while saving record. Please try again.", "auto"); - } else { + try { + $model->store($is_new); $msg = $is_new ? _T("New model has been added!", "auto") : _T("Model has been saved!", "auto"); $this->flash->addMessage( 'success_detected', $msg ); + } catch (\Throwable $e) { + $error_detected[] + = _T("- An error occurred while saving record. Please try again.", "auto"); } } @@ -303,6 +303,22 @@ public function formUri(array $args): string ); } + /** + * Removal confirmation parameters, for existing models only + * + * @return array + * + * @throws HttpNotFoundException + */ + protected function getconfirmDeleteParams(Request $request): array + { + $args = $this->getArgs($request); + if (!isset($args['ids']) && !(new Model($this->zdb))->load((int)$args['id'])) { + throw new HttpNotFoundException($request); + } + return parent::getconfirmDeleteParams($request); + } + /** * Get confirmation removal page title * diff --git a/lib/GaletteAuto/Controllers/Crud/PropertiesController.php b/lib/GaletteAuto/Controllers/Crud/PropertiesController.php index ed095ca..cffae84 100644 --- a/lib/GaletteAuto/Controllers/Crud/PropertiesController.php +++ b/lib/GaletteAuto/Controllers/Crud/PropertiesController.php @@ -19,6 +19,7 @@ use GaletteAuto\Finition; use GaletteAuto\State; use GaletteAuto\Transmission; +use Slim\Exception\HttpNotFoundException; use Slim\Psr7\Request; use Slim\Psr7\Response; use GaletteAuto\Filters\ModelsList; @@ -220,7 +221,7 @@ public function filter(Request $request, Response $response, string $property): */ public function propertyAdd(Request $request, Response $response, string $property): Response { - return $this->propertyEdit($response, $property, null, 'add'); + return $this->propertyEdit($request, $response, $property, null, 'add'); } /** @@ -230,15 +231,20 @@ public function propertyAdd(Request $request, Response $response, string $proper * @param ?int $id Property ID, if any * @param string $action 'add' or 'edit' */ - public function propertyEdit(Response $response, string $property, ?int $id = null, string $action = 'edit'): Response - { + public function propertyEdit( + Request $request, + Response $response, + string $property, + ?int $id = null, + string $action = 'edit' + ): Response { $is_new = ($action === 'add'); $object = AbstractObject::fromPropertyName($this->zdb, $property); if ($is_new) { $title = _T("New", "auto"); } else { - $object->load($id); + $this->loadOrNotFound($request, $object, (int)$id); $title = str_replace( '%s', $object->getValue(), @@ -303,9 +309,8 @@ public function doPropertyEdit( $error_detected = []; - if (!$is_new && !$object->load((int)$id)) { - $error_detected[] - = _T("- An error occurred while saving record. Please try again.", "auto"); + if (!$is_new) { + $this->loadOrNotFound($request, $object, (int)$id); } $value = $post[$object->getField()] ?? null; @@ -316,11 +321,8 @@ public function doPropertyEdit( } if (count($error_detected) == 0) { - $res = $object->store($is_new); - if (!$res) { - $error_detected[] - = _T("- An error occurred while saving record. Please try again.", "auto"); - } else { + try { + $object->store($is_new); $msg = str_replace( '%property', $object->getFieldLabel(), @@ -331,6 +333,9 @@ public function doPropertyEdit( 'success_detected', $msg ); + } catch (\Throwable $e) { + $error_detected[] + = _T("- An error occurred while saving record. Please try again.", "auto"); } } @@ -371,10 +376,10 @@ public function doPropertyEdit( * @param string $property Property name * @param int $id Property ID, if any */ - public function propertyShow(Response $response, string $property, int $id): Response + public function propertyShow(Request $request, Response $response, string $property, int $id): Response { $object = AbstractObject::fromPropertyName($this->zdb, $property); - $object->load($id); + $this->loadOrNotFound($request, $object, $id); $title = str_replace( '%s', $object->getValue(), @@ -414,7 +419,7 @@ public function propertyShow(Response $response, string $property, int $id): Res public function removeProperty(Request $request, Response $response, string $property, int $id): Response { $object = AbstractObject::fromPropertyName($this->zdb, $property); - $object->load($id); + $this->loadOrNotFound($request, $object, $id); $route = $object::getListRoute($this->routeparser); @@ -502,6 +507,21 @@ public function doRemoveProperty(Request $request, Response $response, string $p } } + /** + * Load a property, or answer with a not found error + * + * @param AbstractObject $object Property instance + * @param int $id Property ID + * + * @throws HttpNotFoundException + */ + protected function loadOrNotFound(Request $request, AbstractObject $object, int $id): void + { + if (!$object->load($id)) { + throw new HttpNotFoundException($request); + } + } + /** * Get properties repository * diff --git a/lib/GaletteAuto/History.php b/lib/GaletteAuto/History.php index 8b85eeb..11e4c11 100644 --- a/lib/GaletteAuto/History.php +++ b/lib/GaletteAuto/History.php @@ -90,10 +90,9 @@ public function load(int $id): bool $results = $this->zdb->execute($select); $this->formatEntries($results->toArray()); return true; - } catch (\Exception $e) { + } catch (\Throwable $e) { Analog::log( - '[' . get_class($this) . '] Cannot get car\'s history (id was ' - . $this->id_car . ') | ' . $e->getMessage(), + '[' . static::class . '] Cannot load history of vehicle #' . $this->id_car . ' | ' . $e->getMessage(), Analog::ERROR ); return false; @@ -121,11 +120,10 @@ public function getLatest(): ArrayObject|false } else { return false; } - } catch (\Exception $e) { + } catch (\Throwable $e) { Analog::log( - '[' . get_class($this) - . '] Cannot get car\'s latest history entry | ' - . $e->getMessage(), + '[' . static::class . '] Cannot load latest history entry of vehicle #' . $this->id_car + . ' | ' . $e->getMessage(), Analog::ERROR ); return false; @@ -176,10 +174,10 @@ public function register(array $props): void 'An error occurred registering car new history entry :(' ); } - } catch (\Exception $e) { + } catch (\Throwable $e) { Analog::log( - '[' . get_class($this) . '] Cannot register new history entry | ' - . $e->getMessage(), + '[' . static::class . '] Cannot add history entry of vehicle #' . ($props[Auto::PK] ?? '') + . ' | ' . $e->getMessage(), Analog::ERROR ); throw $e; diff --git a/lib/GaletteAuto/Model.php b/lib/GaletteAuto/Model.php index b5f99ac..fdc7490 100644 --- a/lib/GaletteAuto/Model.php +++ b/lib/GaletteAuto/Model.php @@ -74,10 +74,9 @@ public function load(int $id): bool } $this->loadFromRS($result); return true; - } catch (\Exception $e) { + } catch (\Throwable $e) { Analog::log( - '[' . get_class($this) . '] Cannot load model from id `' . $id - . '` | ' . $e->getMessage(), + '[' . static::class . '] Cannot load model #' . $id . ' | ' . $e->getMessage(), Analog::ERROR ); return false; @@ -105,8 +104,10 @@ private function loadFromRS(ArrayObject $r): void * Store current model * * @param bool $new New record or existing one + * + * @throws \Throwable */ - public function store(bool $new = false): bool + public function store(bool $new = false): void { try { $values = [ @@ -132,15 +133,13 @@ public function store(bool $new = false): bool ); $this->zdb->execute($update); } - return true; - } catch (\Exception $e) { + } catch (\Throwable $e) { Analog::log( - '[' . get_class($this) . '] Cannot store model' - . ' values `' . $this->id . '`, `' . implode('`, `', $values) . '` | ' - . $e->getMessage(), - Analog::WARNING + '[' . static::class . '] Cannot ' . ($new ? 'add' : 'update') . ' model #' . ($this->id ?? '') + . ' | ' . $e->getMessage(), + Analog::ERROR ); - return false; + throw $e; } } diff --git a/lib/GaletteAuto/Repository/Vehicles.php b/lib/GaletteAuto/Repository/Vehicles.php index 65b17f4..5b46109 100644 --- a/lib/GaletteAuto/Repository/Vehicles.php +++ b/lib/GaletteAuto/Repository/Vehicles.php @@ -211,7 +211,7 @@ public function store(Auto $vehicle): void $this->zdb->rollback(); } Analog::log( - 'Unable to ' . ($new ? 'add' : 'update') . ' vehicle #' . ($vehicle->getId() ?? '') + '[' . static::class . '] Cannot ' . ($new ? 'add' : 'update') . ' vehicle #' . ($vehicle->getId() ?? '') . ' | ' . $e->getMessage(), Analog::ERROR ); @@ -287,7 +287,7 @@ public function remove(array $ids): void $this->zdb->rollback(); } Analog::log( - 'Unable to remove vehicles #' . implode(', #', $ids) . ' | ' . $e->getMessage(), + '[' . static::class . '] Cannot remove vehicles #' . implode(', #', $ids) . ' | ' . $e->getMessage(), Analog::ERROR ); throw $e; diff --git a/tests/GaletteAuto/Controllers/tests/units/Controller.php b/tests/GaletteAuto/Controllers/tests/units/Controller.php index 6dfafd0..b566e3a 100644 --- a/tests/GaletteAuto/Controllers/tests/units/Controller.php +++ b/tests/GaletteAuto/Controllers/tests/units/Controller.php @@ -47,13 +47,13 @@ public function setUp(): void $class = '\GaletteAuto\\' . ucfirst($property); $object = new $class($this->zdb); $object->setValue($value); - $this->assertTrue($object->store(true)); + $object->store(true); $this->props[$property] = $object->getId(); } $model = new \GaletteAuto\Model($this->zdb); $this->assertTrue($model->check(['model' => '307', 'brand' => $this->props['brand']])); - $this->assertTrue($model->store(true)); + $model->store(true); $this->props['model'] = $model->getId(); } @@ -250,7 +250,7 @@ public function testShowMissingVehicle(): void $this->logSuperAdmin(); $request = $this->createRequest('vehicleEdit', ['id' => '999999']); $test_response = $this->app->handle($request); - $this->expectLogEntry(Analog::ERROR, 'Cannot load car from id `999999`'); + $this->expectLogEntry(Analog::ERROR, 'Cannot load vehicle #999999 |'); $this->expectAccessDenied($test_response, 'Trying to edit vehicle #999999'); } @@ -994,9 +994,9 @@ public function testAjaxModels(): void $model = new \GaletteAuto\Model($this->zdb); $brand = new \GaletteAuto\Brand($this->zdb); $brand->setValue('Renault'); - $this->assertTrue($brand->store(true)); + $brand->store(true); $this->assertTrue($model->check(['model' => 'Clio', 'brand' => $brand->getId()])); - $this->assertTrue($model->store(true)); + $model->store(true); $this->getMemberOne(); $this->logMember($this->dataAdherentOne()); diff --git a/tests/GaletteAuto/Controllers/tests/units/ModelsController.php b/tests/GaletteAuto/Controllers/tests/units/ModelsController.php index d78bf07..c92f01e 100644 --- a/tests/GaletteAuto/Controllers/tests/units/ModelsController.php +++ b/tests/GaletteAuto/Controllers/tests/units/ModelsController.php @@ -34,7 +34,7 @@ public function setUp(): void parent::setUp(); $brand = new Brand($this->zdb); $brand->setValue('Peugeot'); - $this->assertTrue($brand->store(true)); + $brand->store(true); $this->brand_id = $brand->getId(); } @@ -56,7 +56,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)); + $model->store(true); return $model->getId(); } @@ -129,7 +129,7 @@ private function createVehicle(int $model_id): void $class = '\\GaletteAuto\\' . $property; $object = new $class($this->zdb); $object->setValue('Test ' . $property); - $this->assertTrue($object->store(true)); + $object->store(true); $values[$class::PK] = $object->getId(); } $insert = $this->zdb->insert(AUTO_PREFIX . \GaletteAuto\Auto::TABLE); @@ -203,6 +203,26 @@ public function testMemberAccess(): void $this->expectAuthMiddlewareRefused($this->app->handle($this->createRequest('modelsList'))); } + /** + * A model that does not exist is not found + */ + public function testMissingModel(): void + { + $this->logSuperAdmin(); + $requests = [ + $this->createRequest('modelEdit', ['id' => '999999']), + $this->createRequest('removeModel', ['id' => '999999']), + $this->createRequest('doModelEdit', ['id' => '999999'], 'POST') + ->withParsedBody(['model' => '307']), + ]; + foreach ($requests as $request) { + $test_response = $this->app->handle($request); + $this->assertSame(404, $test_response->getStatusCode()); + } + $this->expectLogEntry(\Analog\Analog::ERROR, 'Cannot load model #999999 | Model not found'); + $this->expectNoLogEntry(); + } + /** * Remove models */ @@ -226,7 +246,7 @@ public function testRemove(): void ); $this->expectFlashData(['success_detected' => ['Successfully deleted!']]); $this->assertFalse((new Model($this->zdb))->load($unused)); - $this->expectLogEntry(\Analog\Analog::ERROR, 'Cannot load model from id `' . $unused . '`'); + $this->expectLogEntry(\Analog\Analog::ERROR, 'Cannot load model #' . $unused . ' | Model not found'); //used model cannot be removed; last check, pgsql aborts the transaction $request = $this->createRequest('doRemoveModel', ['id' => (string)$used], 'POST') diff --git a/tests/GaletteAuto/Controllers/tests/units/PropertiesController.php b/tests/GaletteAuto/Controllers/tests/units/PropertiesController.php index 6d898b2..17f5a6e 100644 --- a/tests/GaletteAuto/Controllers/tests/units/PropertiesController.php +++ b/tests/GaletteAuto/Controllers/tests/units/PropertiesController.php @@ -41,7 +41,7 @@ private function createColor(string $value): int { $color = new Color($this->zdb); $color->setValue($value); - $this->assertTrue($color->store(true)); + $color->store(true); return $color->getId(); } @@ -81,6 +81,28 @@ public function testEditUsesRouteId(): void $this->assertSame('Blue', $this->getColor($blue)); } + /** + * A property that does not exist is not found + */ + public function testMissingProperty(): void + { + $this->logSuperAdmin(); + $requests = [ + $this->createRequest('propertyEdit', ['property' => 'color', 'id' => '999999']), + $this->createRequest('propertyShow', ['property' => 'brand', 'id' => '999999']), + $this->createRequest('removeProperty', ['property' => 'color', 'id' => '999999']), + $this->createRequest('doPropertyEdit', ['property' => 'color', 'id' => '999999'], 'POST') + ->withParsedBody(['color' => 'Red']), + ]; + foreach ($requests as $request) { + $test_response = $this->app->handle($request); + $this->assertSame(404, $test_response->getStatusCode()); + } + $this->expectLogEntry(\Analog\Analog::ERROR, 'Cannot load color #999999 | Record not found'); + $this->expectLogEntry(\Analog\Analog::ERROR, 'Cannot load brand #999999 | Record not found'); + $this->expectNoLogEntry(); + } + /** * Adding an empty property goes back to add form */ @@ -132,7 +154,7 @@ public function testLists(): void foreach (['Zeta ' . $property, 'Alpha ' . $property] as $value) { $object = new $class($this->zdb); $object->setValue($value); - $this->assertTrue($object->store(true)); + $object->store(true); } $test_response = $this->app->handle($this->createRequest($route)); @@ -167,10 +189,10 @@ public function testEditAndShow(): void $id = $this->createColor('Red'); $brand = new \GaletteAuto\Brand($this->zdb); $brand->setValue('Peugeot'); - $this->assertTrue($brand->store(true)); + $brand->store(true); $model = new \GaletteAuto\Model($this->zdb); $this->assertTrue($model->check(['model' => '307', 'brand' => $brand->getId()])); - $this->assertTrue($model->store(true)); + $model->store(true); $this->logSuperAdmin(); $test_response = $this->app->handle( @@ -221,15 +243,15 @@ public function testRemove(): void $class = '\\GaletteAuto\\' . $property; $object = new $class($this->zdb); $object->setValue('Test ' . $property); - $this->assertTrue($object->store(true)); + $object->store(true); $values[$class::PK] = $object->getId(); } $brand = new \GaletteAuto\Brand($this->zdb); $brand->setValue('Peugeot'); - $this->assertTrue($brand->store(true)); + $brand->store(true); $model = new \GaletteAuto\Model($this->zdb); $this->assertTrue($model->check(['model' => '307', 'brand' => $brand->getId()])); - $this->assertTrue($model->store(true)); + $model->store(true); $insert = $this->zdb->insert(AUTO_PREFIX . \GaletteAuto\Auto::TABLE); $insert->values($values + [ 'car_name' => 'Titine', diff --git a/tests/GaletteAuto/tests/units/Auto.php b/tests/GaletteAuto/tests/units/Auto.php index 4c7c9b2..256d3b9 100644 --- a/tests/GaletteAuto/tests/units/Auto.php +++ b/tests/GaletteAuto/tests/units/Auto.php @@ -39,32 +39,32 @@ public function testCrud(): void { $body = new \GaletteAuto\Body($this->zdb); $body->setValue('Berline'); - $this->assertTrue($body->store(true)); + $body->store(true); $body_id = $body->getId(); $color = new \GaletteAuto\Color($this->zdb); $color->setValue('Grey'); - $this->assertTrue($color->store(true)); + $color->store(true); $color_id = $color->getId(); $finition = new \GaletteAuto\Finition($this->zdb); $finition->setValue('Standard'); - $this->assertTrue($finition->store(true)); + $finition->store(true); $finition_id = $finition->getId(); $state = new \GaletteAuto\State($this->zdb); $state->setValue('Correct'); - $this->assertTrue($state->store(true)); + $state->store(true); $state_id = $state->getId(); $transmission = new \GaletteAuto\Transmission($this->zdb); $transmission->setValue('Manual'); - $this->assertTrue($transmission->store(true)); + $transmission->store(true); $transmission_id = $transmission->getId(); $brand = new \GaletteAuto\Brand($this->zdb); $brand->setValue('Peugeot'); - $this->assertTrue($brand->store(true)); + $brand->store(true); $brand_id = $brand->getId(); $model = new \GaletteAuto\Model($this->zdb); @@ -73,7 +73,7 @@ public function testCrud(): void 'brand' => $brand_id, ]; $this->assertTrue($model->check($data)); - $this->assertTrue($model->store(true)); + $model->store(true); $model_id = $model->getId(); $this->logSuperAdmin(); @@ -181,7 +181,7 @@ public function testCrud(): void $adh2 = $this->getMemberTwo(); $color2 = new \GaletteAuto\Color($this->zdb); $color2->setValue('Yellow'); - $this->assertTrue($color2->store(true)); + $color2->store(true); $color2_id = $color2->getId(); $data = [ @@ -258,7 +258,7 @@ public function testCrud(): void $this->assertFalse($auto->load($auto_id)); $this->expectLogEntry( \Analog\Analog::ERROR, - '[GaletteAuto\Auto] Cannot load car from id `' . $auto_id . '` | Vehicle not found' + '[GaletteAuto\Auto] Cannot load vehicle #' . $auto_id . ' | Vehicle not found' ); } @@ -281,7 +281,7 @@ public function testLoadError(): void $this->assertFalse($auto->load(999)); $this->expectLogEntry( \Analog\Analog::ERROR, - '[GaletteAuto\Auto] Cannot load car from id `999` | Vehicle not found' + '[GaletteAuto\Auto] Cannot load vehicle #999 | Vehicle not found' ); } } diff --git a/tests/GaletteAuto/tests/units/Body.php b/tests/GaletteAuto/tests/units/Body.php index b3c5265..673ac49 100644 --- a/tests/GaletteAuto/tests/units/Body.php +++ b/tests/GaletteAuto/tests/units/Body.php @@ -56,7 +56,7 @@ public function testCrud(): void //Add new body $body->setValue('Coupe'); - $this->assertTrue($body->store(true)); + $body->store(true); $first_id = $body->getId(); $this->assertCount(1, $bodies->getList()); @@ -69,7 +69,7 @@ public function testCrud(): void //add another one $body = new \GaletteAuto\Body($this->zdb); $body->setValue('Brea'); - $this->assertTrue($body->store(true)); + $body->store(true); $id = $body->getId(); $this->assertCount(2, $bodies->getList()); @@ -78,7 +78,7 @@ public function testCrud(): void $body = new \GaletteAuto\Body($this->zdb); $this->assertTrue($body->load($id)); $body->setValue('Break'); - $this->assertTrue($body->store()); + $body->store(); $this->assertCount(2, $bodies->getList()); $this->assertSame('2 bodies', $body->getCountLabel($bodies->getCount())); @@ -100,7 +100,7 @@ public function testLoadError(): void $this->assertFalse($body->load(999)); $this->expectLogEntry( \Analog\Analog::ERROR, - '[GaletteAuto\Body] Cannot load bodies from id `999`', + '[GaletteAuto\Body] Cannot load body #999 | Record not found', ); } diff --git a/tests/GaletteAuto/tests/units/Brand.php b/tests/GaletteAuto/tests/units/Brand.php index 047950e..74f6609 100644 --- a/tests/GaletteAuto/tests/units/Brand.php +++ b/tests/GaletteAuto/tests/units/Brand.php @@ -56,7 +56,7 @@ public function testCrud(): void //Add new brand $brand->setValue('Audi'); - $this->assertTrue($brand->store(true)); + $brand->store(true); $first_id = $brand->getId(); $this->assertCount(1, $brands->getList()); @@ -69,7 +69,7 @@ public function testCrud(): void //add another one $brand = new \GaletteAuto\Brand($this->zdb); $brand->setValue('Mercede'); - $this->assertTrue($brand->store(true)); + $brand->store(true); $id = $brand->getId(); $this->assertCount(2, $brands->getList()); @@ -78,7 +78,7 @@ public function testCrud(): void $brand = new \GaletteAuto\Brand($this->zdb); $this->assertTrue($brand->load($id)); $brand->setValue('Mercedes'); - $this->assertTrue($brand->store()); + $brand->store(); $this->assertCount(2, $brands->getList()); $this->assertSame('2 brands', $brand->getCountLabel($brands->getCount())); @@ -100,7 +100,7 @@ public function testLoadError(): void $this->assertFalse($brand->load(999)); $this->expectLogEntry( \Analog\Analog::ERROR, - '[GaletteAuto\Brand] Cannot load brands from id `999`' + '[GaletteAuto\Brand] Cannot load brand #999 | Record not found' ); } diff --git a/tests/GaletteAuto/tests/units/Color.php b/tests/GaletteAuto/tests/units/Color.php index 18b0904..127d430 100644 --- a/tests/GaletteAuto/tests/units/Color.php +++ b/tests/GaletteAuto/tests/units/Color.php @@ -56,7 +56,7 @@ public function testCrud(): void //Add new color $color->setValue('Red'); - $this->assertTrue($color->store(true)); + $color->store(true); $first_id = $color->getId(); $this->assertCount(1, $colors->getList()); @@ -69,7 +69,7 @@ public function testCrud(): void //add another one $color = new \GaletteAuto\Color($this->zdb); $color->setValue('Blu'); - $this->assertTrue($color->store(true)); + $color->store(true); $id = $color->getId(); $this->assertCount(2, $colors->getList()); @@ -80,7 +80,7 @@ public function testCrud(): void $color = new \GaletteAuto\Color($this->zdb); $this->assertTrue($color->load($id)); $color->setValue('Blue'); - $this->assertTrue($color->store()); + $color->store(); $this->assertCount(2, $colors->getList()); $this->assertSame('2 colors', $color->getCountLabel($colors->getCount())); @@ -102,7 +102,7 @@ public function testLoadError(): void $this->assertFalse($color->load(999)); $this->expectLogEntry( \Analog\Analog::ERROR, - '[GaletteAuto\Color] Cannot load colors from id `999`' + '[GaletteAuto\Color] Cannot load color #999 | Record not found' ); } diff --git a/tests/GaletteAuto/tests/units/Finition.php b/tests/GaletteAuto/tests/units/Finition.php index e6308b8..e2b1821 100644 --- a/tests/GaletteAuto/tests/units/Finition.php +++ b/tests/GaletteAuto/tests/units/Finition.php @@ -56,7 +56,7 @@ public function testCrud(): void //Add new finition $finition->setValue('Feline'); - $this->assertTrue($finition->store(true)); + $finition->store(true); $first_id = $finition->getId(); $this->assertCount(1, $finitions->getList()); @@ -69,7 +69,7 @@ public function testCrud(): void //add another one $finition = new \GaletteAuto\Finition($this->zdb); $finition->setValue('R'); - $this->assertTrue($finition->store(true)); + $finition->store(true); $id = $finition->getId(); $this->assertCount(2, $finitions->getList()); @@ -78,7 +78,7 @@ public function testCrud(): void $finition = new \GaletteAuto\Finition($this->zdb); $this->assertTrue($finition->load($id)); $finition->setValue('RS'); - $this->assertTrue($finition->store()); + $finition->store(); $this->assertCount(2, $finitions->getList()); $this->assertSame('2 finitions', $finition->getCountLabel($finitions->getCount())); @@ -100,7 +100,7 @@ public function testLoadError(): void $this->assertFalse($finition->load(999)); $this->expectLogEntry( \Analog\Analog::ERROR, - '[GaletteAuto\Finition] Cannot load finitions from id `999`' + '[GaletteAuto\Finition] Cannot load finition #999 | Record not found' ); } diff --git a/tests/GaletteAuto/tests/units/Model.php b/tests/GaletteAuto/tests/units/Model.php index 5d1ba3c..04866ee 100644 --- a/tests/GaletteAuto/tests/units/Model.php +++ b/tests/GaletteAuto/tests/units/Model.php @@ -29,13 +29,13 @@ public function testCrud(): void $brand = new \GaletteAuto\Brand($this->zdb); //Add new brand $brand->setValue('Audi'); - $this->assertTrue($brand->store(true)); + $brand->store(true); $first_brand_id = $brand->getId(); //add another brand $brand = new \GaletteAuto\Brand($this->zdb); $brand->setValue('Mercedes'); - $this->assertTrue($brand->store(true)); + $brand->store(true); $second_brand_id = $brand->getId(); $brands = new \GaletteAuto\Repository\Properties( @@ -92,7 +92,7 @@ public function testCrud(): void 'brand' => $first_brand_id, ]; $this->assertTrue($model->check($data)); - $this->assertTrue($model->store(true)); + $model->store(true); $this->assertCount(1, $models->getList()); $this->assertCount(1, $models->getList($first_brand_id)); @@ -104,7 +104,7 @@ public function testCrud(): void 'brand' => $first_brand_id, ]; $this->assertTrue($model->check($data)); - $this->assertTrue($model->store(true)); + $model->store(true); $id_model = $model->getId(); $this->assertCount(2, $models->getList()); @@ -114,7 +114,7 @@ public function testCrud(): void 'brand' => $first_brand_id ]; $this->assertTrue($model->check($data)); - $this->assertTrue($model->store()); + $model->store(); $this->assertCount(2, $models->getList()); $this->assertCount(2, $models->getList($first_brand_id)); @@ -126,7 +126,7 @@ public function testCrud(): void 'brand' => $second_brand_id, ]; $this->assertTrue($model->check($data)); - $this->assertTrue($model->store(true)); + $model->store(true); $this->assertCount(3, $models->getList()); $this->assertSame(3, $models->getCount()); @@ -153,7 +153,7 @@ public function testLoadError(): void $this->assertFalse($brand->load(999)); $this->expectLogEntry( \Analog\Analog::ERROR, - '[GaletteAuto\Model] Cannot load model from id `999`' + '[GaletteAuto\Model] Cannot load model #999 | Model not found' ); } } diff --git a/tests/GaletteAuto/tests/units/State.php b/tests/GaletteAuto/tests/units/State.php index 000aff1..dbb454a 100644 --- a/tests/GaletteAuto/tests/units/State.php +++ b/tests/GaletteAuto/tests/units/State.php @@ -56,7 +56,7 @@ public function testCrud(): void //Add new state $state->setValue('Good'); - $this->assertTrue($state->store(true)); + $state->store(true); $first_id = $state->getId(); $this->assertCount(1, $states->getList()); @@ -69,7 +69,7 @@ public function testCrud(): void //add another one $state = new \GaletteAuto\State($this->zdb); $state->setValue('Wrec'); - $this->assertTrue($state->store(true)); + $state->store(true); $id = $state->getId(); $this->assertCount(2, $states->getList()); @@ -78,7 +78,7 @@ public function testCrud(): void $state = new \GaletteAuto\State($this->zdb); $this->assertTrue($state->load($id)); $state->setValue('Wreck'); - $this->assertTrue($state->store()); + $state->store(); $this->assertCount(2, $states->getList()); $this->assertSame('2 states', $state->getCountLabel($states->getCount())); @@ -100,7 +100,7 @@ public function testLoadError(): void $this->assertFalse($state->load(999)); $this->expectLogEntry( \Analog\Analog::ERROR, - '[GaletteAuto\State] Cannot load states from id `999`' + '[GaletteAuto\State] Cannot load state #999 | Record not found' ); } diff --git a/tests/GaletteAuto/tests/units/Transmission.php b/tests/GaletteAuto/tests/units/Transmission.php index dfe8a3c..fdb0bd2 100644 --- a/tests/GaletteAuto/tests/units/Transmission.php +++ b/tests/GaletteAuto/tests/units/Transmission.php @@ -56,7 +56,7 @@ public function testCrud(): void //Add new transmission $transmission->setValue('Manual'); - $this->assertTrue($transmission->store(true)); + $transmission->store(true); $first_id = $transmission->getId(); $this->assertCount(1, $transmissions->getList()); @@ -69,7 +69,7 @@ public function testCrud(): void //add another one $transmission = new \GaletteAuto\Transmission($this->zdb); $transmission->setValue('Auto'); - $this->assertTrue($transmission->store(true)); + $transmission->store(true); $id = $transmission->getId(); $this->assertCount(2, $transmissions->getList()); @@ -78,7 +78,7 @@ public function testCrud(): void $transmission = new \GaletteAuto\Transmission($this->zdb); $this->assertTrue($transmission->load($id)); $transmission->setValue('Automatic'); - $this->assertTrue($transmission->store()); + $transmission->store(); $this->assertCount(2, $transmissions->getList()); $this->assertSame('2 transmissions', $transmission->getCountLabel($transmissions->getCount())); @@ -100,7 +100,7 @@ public function testLoadError(): void $this->assertFalse($transmission->load(999)); $this->expectLogEntry( \Analog\Analog::ERROR, - '[GaletteAuto\Transmission] Cannot load transmissions from id `999`' + '[GaletteAuto\Transmission] Cannot load transmission #999 | Record not found' ); }