From acbb5cf06131134a1852b6ed69987a3d3bcd95f1 Mon Sep 17 00:00:00 2001 From: Donovan Date: Tue, 29 Dec 2020 17:27:42 +0100 Subject: [PATCH] =?UTF-8?q?Am=C3=A9liore=20la=20validation=20(+=20Refactor?= =?UTF-8?q?ing=20mod=C3=A8les)=20(#76)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- CHANGELOG.md | 1 + .../src/App/Controllers/TokenController.php | 2 +- server/src/App/Models/Attribute.php | 8 +- server/src/App/Models/BaseModel.php | 179 ++++++++++-------- server/src/App/Models/Bill.php | 17 +- server/src/App/Models/Category.php | 57 +++--- server/src/App/Models/Company.php | 12 +- server/src/App/Models/Country.php | 12 +- server/src/App/Models/Event.php | 47 ++--- server/src/App/Models/Material.php | 11 +- server/src/App/Models/Park.php | 11 +- server/src/App/Models/Person.php | 44 ++--- server/src/App/Models/SubCategory.php | 11 +- server/src/App/Models/Tag.php | 59 +++--- server/src/App/Models/Traits/Taggable.php | 2 +- server/src/App/Models/Traits/WithPdf.php | 4 +- server/src/App/Models/User.php | 12 +- server/src/App/Models/UserSetting.php | 8 +- .../Exceptions/CallbackException.php | 9 + server/src/App/Validation/Rules/Callback.php | 39 ++++ server/src/App/Validation/Validator.php | 45 +++-- server/tests/models/AttributeTest.php | 5 + server/tests/models/BillTest.php | 5 + server/tests/models/CategoryTest.php | 14 +- server/tests/models/CompanyTest.php | 5 + server/tests/models/CountryTest.php | 5 + server/tests/models/EventTest.php | 13 +- server/tests/models/MaterialTest.php | 5 + server/tests/models/ParkTest.php | 5 + server/tests/models/PersonTest.php | 5 + server/tests/models/SubCategoryTest.php | 5 + server/tests/models/TagTest.php | 10 +- server/tests/models/UserSettingTest.php | 5 + server/tests/models/UserTest.php | 5 + 34 files changed, 384 insertions(+), 293 deletions(-) create mode 100644 server/src/App/Validation/Exceptions/CallbackException.php create mode 100644 server/src/App/Validation/Rules/Callback.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 481dbc594..0afa66662 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ Ce projet adhère au principe du [Semantic Versioning](https://semver.org/spec/v Ceci va, par exemple, permettre de simplifier les mises à jour de la version compilée du client (via un simple `yarn build`). (Un lien symbolique est utilisé côté serveur pour relier les deux côtés de l'application) - Corrige l'hôte de développement et permet sa customisation via une variable d'environnement. +- Améliorations internes de la validation des données. ## 0.10.2 (2020-11-16) diff --git a/server/src/App/Controllers/TokenController.php b/server/src/App/Controllers/TokenController.php index c80012bea..b85d652bc 100644 --- a/server/src/App/Controllers/TokenController.php +++ b/server/src/App/Controllers/TokenController.php @@ -5,7 +5,7 @@ use Slim\Http\Request; use Slim\Http\Response; -use Respect\Validation\Validator as V; +use Robert2\API\Validation\Validator as V; use Robert2\API\Errors\ValidationException; use Robert2\API\Middlewares\Security; diff --git a/server/src/App/Models/Attribute.php b/server/src/App/Models/Attribute.php index 8f0ae23eb..2a7348748 100644 --- a/server/src/App/Models/Attribute.php +++ b/server/src/App/Models/Attribute.php @@ -3,15 +3,11 @@ namespace Robert2\API\Models; -use Respect\Validation\Validator as V; +use Robert2\API\Validation\Validator as V; class Attribute extends BaseModel { - protected $table = 'attributes'; - - protected $_modelName = 'Attribute'; - protected $_orderField = 'id'; - protected $_orderDirection = 'asc'; + protected $orderField = 'id'; public function __construct(array $attributes = []) { diff --git a/server/src/App/Models/BaseModel.php b/server/src/App/Models/BaseModel.php index 91e56e4b7..9918e7eee 100644 --- a/server/src/App/Models/BaseModel.php +++ b/server/src/App/Models/BaseModel.php @@ -6,23 +6,21 @@ use Illuminate\Database\Eloquent\Model; use Illuminate\Database\QueryException; use Illuminate\Database\Eloquent\Builder; +use Respect\Validation\Exceptions\NestedValidationException; use InvalidArgumentException; -use Robert2\API\Validation\Validator; use Robert2\API\Config\Config; use Robert2\API\Errors; class BaseModel extends Model { - protected $table; - protected $_settings; + private $columns; - protected $_modelName; - protected $_orderField; - protected $_orderDirection; + protected $orderField; + protected $orderDirection; - protected $_allowedSearchFields; - protected $_searchField; - protected $_searchTerm; + protected $allowedSearchFields = []; + protected $searchField; + protected $searchTerm; protected $fillable; @@ -32,17 +30,12 @@ class BaseModel extends Model 'deleted_at', ]; - private $_validator; - public $validation; const EXTRA_CHARS = '-_. ÇçàÀâÂäÄåÅèÈéÉêÊëËíÍìÌîÎïÏòÒóÓôÔöÖðÐõÕøØúÚùÙûÛüÜýÝÿŸŷŶøØæÆœŒñÑßÞ'; public function __construct(array $attributes = []) { - $this->_settings = Config::getSettings(); - $this->_validator = new Validator(); - Config::getCapsule(); parent::__construct($attributes); @@ -58,7 +51,7 @@ public function getAll(bool $withDeleted = false): Builder { $builder = $this->_getOrderBy(); - if (!empty($this->_searchTerm)) { + if (!empty($this->searchTerm)) { $builder = $this->_setSearchConditions($builder); } @@ -73,7 +66,7 @@ public function getAllFiltered(array $conditions, bool $withDeleted = false): Bu { $builder = self::where($conditions); - if (!empty($this->_searchTerm)) { + if (!empty($this->searchTerm)) { $builder = $this->_setSearchConditions($builder); } @@ -84,25 +77,57 @@ public function getAllFiltered(array $conditions, bool $withDeleted = false): Bu return $this->_getOrderBy($builder); } + // ------------------------------------------------------ + // - + // - Setters + // - + // ------------------------------------------------------ + + public function setOrderBy(?string $orderBy = null, bool $ascending = true): BaseModel + { + if ($orderBy) { + $this->orderField = $orderBy; + } + $this->orderDirection = $ascending ? 'asc' : 'desc'; + return $this; + } + + public function setSearch(?string $term = null, $fields = null): BaseModel + { + if (empty($term)) { + return $this; + } + + if ($fields) { + $fields = !is_array($fields) ? explode('|', $fields) : $fields; + foreach ($fields as $field) { + if (!in_array($field, $this->getAllowedSearchFields())) { + throw new InvalidArgumentException("Search field « $field » not allowed."); + } + $this->searchField = $field; + } + } + + $this->searchTerm = trim($term); + return $this; + } + // —————————————————————————————————————————————————————— // — - // — Setters + // — "Repository" methods // — // —————————————————————————————————————————————————————— public function edit(?int $id = null, array $data = []): Model { if ($id && !$this->exists($id)) { - throw new Errors\NotFoundException("Edit model $this->_modelName failed, entity not found."); + throw new Errors\NotFoundException(sprintf("Edit failed, record %d not found.", $id)); } $data = cleanEmptyFields($data); - - $onlyFields = $id ? array_keys($data) : []; - $this->validate($data, $onlyFields); - try { - $model = self::updateOrCreate(['id' => $id], $data); + $model = self::firstOrNew(compact('id')); + $model->fill($data)->validate()->save(); } catch (QueryException $e) { $error = new Errors\ValidationException(); $error->setPDOValidationException($e); @@ -125,13 +150,13 @@ public function remove(int $id, array $options = []): ?Model if ($model->trashed() || $options['force'] === true) { if (!$model->forceDelete()) { - throw new \RuntimeException("Unable to destroy $this->_modelName ID #$id."); + throw new \RuntimeException(sprintf("Unable to destroy the record %d.", $id)); } return null; } if (!$model->delete()) { - throw new \RuntimeException("Unable to delete $this->_modelName ID #$id."); + throw new \RuntimeException(sprintf("Unable to delete the record %d.", $id)); } return $model; @@ -145,39 +170,12 @@ public function unremove(int $id): Model } if (!$model->restore()) { - throw new \RuntimeException("Unable to restore $this->_modelName ID #$id."); + throw new \RuntimeException(sprintf("Unable to restore the record %d.", $id)); } return $model; } - public function setOrderBy(?string $orderBy = null, bool $ascending = true): BaseModel - { - if ($orderBy) { - $this->_orderField = $orderBy; - } - $this->_orderDirection = $ascending ? 'asc' : 'desc'; - return $this; - } - - public function setSearch(?string $term = null, ?string $field = null): BaseModel - { - if (empty($term)) { - return $this; - } - - if ($field) { - if (!in_array($field, $this->_allowedSearchFields)) { - throw new InvalidArgumentException("Search field « $field » not allowed."); - } - - $this->_searchField = $field; - } - - $this->_searchTerm = trim($term); - return $this; - } - // —————————————————————————————————————————————————————— // — // — Other useful methods @@ -189,34 +187,60 @@ public function exists(int $id): bool return self::where('id', $id)->exists(); } - public function validate(array $data, array $onlyFields = []): void + public function validate(): self { - if (empty($this->validation)) { - throw new \RuntimeException("Validation rules cannot be empty for model $this->_modelName."); + $rules = $this->validation; + if (empty($rules)) { + throw new \RuntimeException("Validation rules cannot be empty."); } + $data = $this->getAttributes(); foreach ($data as $field => $value) { if (is_array($value)) { unset($data[$field]); } } - if (!empty($onlyFields)) { - foreach (array_keys($this->validation) as $fieldToValidate) { - if (!in_array($fieldToValidate, $onlyFields)) { - unset($this->validation[$fieldToValidate]); - unset($data[$fieldToValidate]); - } - } + // - Si le modèle existe déjà en base, on ne valide que les champs qui ont changé. + if ($this->exists) { + $rules = array_intersect_key($rules, $this->getDirty()); } - $this->_validator->validate($data, $this->validation); + // - Validation + $errors = []; + foreach ($rules as $field => $rule) { + try { + $rule->setName($field)->assert($data[$field] ?? null); + } catch (NestedValidationException $e) { + $errors[$field] = $e->getMessages(); + } + } - if ($this->_validator->hasError()) { + if (count($errors) > 0) { $ex = new Errors\ValidationException(); - $ex->setValidationErrors($this->_validator->getErrors()); + $ex->setValidationErrors($errors); throw $ex; } + + return $this; + } + + public function getTableColumns(): array + { + if (!$this->columns) { + $this->columns = $this->getConnection() + ->getSchemaBuilder() + ->getColumnListing($this->getTable()); + } + return $this->columns; + } + + public function getAllowedSearchFields(): array + { + return array_unique(array_merge( + (array)$this->searchField, + (array)$this->allowedSearchFields + )); } // ------------------------------------------------------ @@ -227,8 +251,12 @@ public function validate(array $data, array $onlyFields = []): void protected function _getOrderBy(?Builder $builder = null): Builder { - $order = $this->_orderField ?: 'id'; - $direction = $this->_orderDirection ?: 'asc'; + $direction = $this->orderDirection ?: 'asc'; + + $order = $this->orderField; + if (!$order) { + $order = in_array('name', $this->getTableColumns()) ? 'name' : 'id'; + } if ($builder) { return $builder->orderBy($order, $direction); @@ -239,24 +267,21 @@ protected function _getOrderBy(?Builder $builder = null): Builder protected function _setSearchConditions(Builder $builder): Builder { - if (!$this->_searchField || !$this->_searchTerm) { + if (!$this->searchField || !$this->searchTerm) { return $builder; } - $term = sprintf('%%%s%%', addcslashes($this->_searchTerm, '%_')); + $term = sprintf('%%%s%%', addcslashes($this->searchTerm, '%_')); - if (preg_match('/\|/', $this->_searchField)) { - $fields = explode('|', $this->_searchField); - - $group = function (Builder $query) use ($fields, $term) { - foreach ($fields as $field) { + if (is_array($this->searchField)) { + $group = function (Builder $query) use ($term) { + foreach ($this->searchField as $field) { $query->orWhere($field, 'LIKE', $term); } }; - return $builder->where($group); } - return $builder->where($this->_searchField, 'LIKE', $term); + return $builder->where($this->searchField, 'LIKE', $term); } } diff --git a/server/src/App/Models/Bill.php b/server/src/App/Models/Bill.php index 70754f82f..88fe616a1 100644 --- a/server/src/App/Models/Bill.php +++ b/server/src/App/Models/Bill.php @@ -5,7 +5,7 @@ use Illuminate\Database\Eloquent\Model; use Illuminate\Database\Eloquent\SoftDeletes; -use Respect\Validation\Validator as V; +use Robert2\API\Validation\Validator as V; use Robert2\API\Errors; use Robert2\API\Config\Config; @@ -18,14 +18,11 @@ class Bill extends BaseModel use SoftDeletes; use WithPdf; - protected $table = 'bills'; + protected $orderField = 'date'; + protected $orderDirection = 'desc'; - protected $_modelName = 'Bill'; - protected $_orderField = 'date'; - protected $_orderDirection = 'desc'; - - protected $_allowedSearchFields = ['number', 'due_amount', 'date']; - protected $_searchField = 'number'; + protected $allowedSearchFields = ['number', 'due_amount', 'date']; + protected $searchField = 'number'; public function __construct(array $attributes = []) { @@ -152,7 +149,7 @@ public function getPdfName(int $id): string { $model = $this->withTrashed()->find($id); if (!$model) { - throw new NotFoundException(sprintf('%s not found.', $this->_modelName)); + throw new NotFoundException(sprintf('Record %d not found.', $id)); } $company = Config::getSettings('companyData'); @@ -160,7 +157,7 @@ public function getPdfName(int $id): string $i18n = new I18n(Config::getSettings('defaultLang')); $fileName = sprintf( '%s-%s-%s-%s.pdf', - $i18n->translate($this->_modelName), + $i18n->translate('Bill'), slugify($company['name']), $model->number, slugify($model->Beneficiary->full_name) diff --git a/server/src/App/Models/Category.php b/server/src/App/Models/Category.php index 973e73dc4..e72916c5b 100644 --- a/server/src/App/Models/Category.php +++ b/server/src/App/Models/Category.php @@ -4,21 +4,15 @@ namespace Robert2\API\Models; use Illuminate\Database\Eloquent\SoftDeletes; -use Respect\Validation\Validator as V; +use Illuminate\Database\QueryException; +use Robert2\API\Validation\Validator as V; use Robert2\API\Errors\ValidationException; class Category extends BaseModel { use SoftDeletes; - protected $table = 'categories'; - - protected $_modelName = 'Category'; - protected $_orderField = 'name'; - protected $_orderDirection = 'asc'; - - protected $_allowedSearchFields = ['name']; - protected $_searchField = 'name'; + protected $searchField = 'name'; public function __construct(array $attributes = []) { @@ -107,24 +101,35 @@ public function getIdsByNames(array $names): array public function bulkAdd(array $categoriesNames = []): array { - $categories = []; - foreach ($categoriesNames as $categoryName) { - $existingCategory = self::getIdsByNames([$categoryName]); - if (!empty($existingCategory)) { - continue; + $categories = array_map( + function ($categoryName) { + $existingCategory = self::where('name', $categoryName)->first(); + if ($existingCategory) { + return $existingCategory; + } + + $category = new static(['name' => trim($categoryName)]); + return tap($category, function ($instance) { + $instance->validate(); + }); + }, + $categoriesNames + ); + + $this->getConnection()->transaction(function () use ($categories) { + try { + foreach ($categories as $category) { + if (!$category->exists || $category->isDirty()) { + $category->save(); + } + } + } catch (QueryException $e) { + $error = new ValidationException(); + $error->setPDOValidationException($e); + throw $error; } + }); - $safeCategory = ['name' => trim($categoryName)]; - $this->validate($safeCategory); - - $categories[] = $safeCategory; - } - - $results = []; - foreach ($categories as $categoryData) { - $results[] = self::edit(null, $categoryData); - } - - return $results; + return $categories; } } diff --git a/server/src/App/Models/Company.php b/server/src/App/Models/Company.php index 4c86f97e2..906648fb2 100644 --- a/server/src/App/Models/Company.php +++ b/server/src/App/Models/Company.php @@ -6,7 +6,7 @@ use Illuminate\Database\Eloquent\Model; use Illuminate\Database\Eloquent\SoftDeletes; use Illuminate\Database\Eloquent\ModelNotFoundException; -use Respect\Validation\Validator as V; +use Robert2\API\Validation\Validator as V; use Robert2\API\Errors; use Robert2\API\Models\Traits\Taggable; @@ -16,14 +16,8 @@ class Company extends BaseModel use SoftDeletes; use Taggable; - protected $table = 'companies'; - - protected $_modelName = 'Company'; - protected $_orderField = 'legal_name'; - protected $_orderDirection = 'asc'; - - protected $_allowedSearchFields = ['legal_name']; - protected $_searchField = 'legal_name'; + protected $orderField = 'legal_name'; + protected $searchField = 'legal_name'; public function __construct(array $attributes = []) { diff --git a/server/src/App/Models/Country.php b/server/src/App/Models/Country.php index 5f61eb2af..97f9e92e0 100644 --- a/server/src/App/Models/Country.php +++ b/server/src/App/Models/Country.php @@ -4,20 +4,14 @@ namespace Robert2\API\Models; use Illuminate\Database\Eloquent\SoftDeletes; -use Respect\Validation\Validator as V; +use Robert2\API\Validation\Validator as V; class Country extends BaseModel { use SoftDeletes; - protected $table = 'countries'; - - protected $_modelName = 'Country'; - protected $_orderField = 'name'; - protected $_orderDirection = 'asc'; - - protected $_allowedSearchFields = ['name', 'code']; - protected $_searchField = 'name'; + protected $allowedSearchFields = ['name', 'code']; + protected $searchField = 'name'; public function __construct(array $attributes = []) { diff --git a/server/src/App/Models/Event.php b/server/src/App/Models/Event.php index 91afc234f..83b58ed3e 100644 --- a/server/src/App/Models/Event.php +++ b/server/src/App/Models/Event.php @@ -5,31 +5,24 @@ use Illuminate\Database\Eloquent\SoftDeletes; use Illuminate\Database\Eloquent\Builder; -use Respect\Validation\Validator as V; - use Robert2\API\Config\Config; use Robert2\API\Models\Material; -use Robert2\API\I18n\I18n; use Robert2\API\Models\Traits\WithPdf; use Robert2\Lib\Domain\EventBill; -use Robert2\API\Errors\ValidationException; +use Robert2\API\Validation\Validator as V; class Event extends BaseModel { use SoftDeletes; use WithPdf; - protected $table = 'events'; - - protected $_modelName = 'Event'; - protected $_orderField = 'start_date'; - protected $_orderDirection = 'asc'; + protected $orderField = 'start_date'; protected $_startDate; protected $_endDate; - protected $_allowedSearchFields = ['title', 'start_date', 'end_date', 'location']; - protected $_searchField = 'title'; + protected $allowedSearchFields = ['title', 'start_date', 'end_date', 'location']; + protected $searchField = 'title'; public function __construct(array $attributes = []) { @@ -47,32 +40,34 @@ public function __construct(array $attributes = []) 'description' => V::optional(V::length(null, 255)), 'reference' => V::oneOf(V::nullType(), V::alnum('.,-/_ ')->length(1, 64)), 'start_date' => V::notEmpty()->date(), - 'end_date' => V::notEmpty()->date(), + 'end_date' => V::callback([$this, 'checkEndDate']), 'is_confirmed' => V::notOptional()->boolType(), 'location' => V::optional(V::length(2, 64)), 'is_billable' => V::optional(V::boolType()), ]; } - public function validate(array $data, array $onlyFields = []): void + // ------------------------------------------------------ + // - + // - Validation + // - + // ------------------------------------------------------ + + public function checkEndDate($value) { - parent::validate($data, $onlyFields); + $dateChecker = V::notEmpty()->date(); + if (!$dateChecker->validate($value)) { + return false; + } - if (!empty($onlyFields) && !in_array('end_date', $onlyFields)) { - return; + if (!$dateChecker->validate($this->start_date)) { + return true; } - $startDate = new \DateTime($data['start_date']); - $endDate = new \DateTime($data['end_date']); + $startDate = new \DateTime($this->start_date); + $endDate = new \DateTime($this->end_date); - if ($startDate >= $endDate) { - $i18n = new I18n; - $ex = new ValidationException(); - $ex->setValidationErrors([ - 'end_date' => [$i18n->translate('endDateMustBeLater')] - ]); - throw $ex; - } + return $startDate < $endDate ?: 'endDateMustBeLater'; } // —————————————————————————————————————————————————————— diff --git a/server/src/App/Models/Material.php b/server/src/App/Models/Material.php index 5d6fa2a11..cb204539b 100644 --- a/server/src/App/Models/Material.php +++ b/server/src/App/Models/Material.php @@ -4,7 +4,7 @@ namespace Robert2\API\Models; use Illuminate\Database\Eloquent\SoftDeletes; -use Respect\Validation\Validator as V; +use Robert2\API\Validation\Validator as V; use Robert2\API\Models\Traits\Taggable; @@ -13,14 +13,7 @@ class Material extends BaseModel use SoftDeletes; use Taggable; - protected $table = 'materials'; - - protected $_modelName = 'Material'; - protected $_orderField = 'name'; - protected $_orderDirection = 'asc'; - - protected $_allowedSearchFields = ['name', 'reference', 'name|reference']; - protected $_searchField = 'name|reference'; + protected $searchField = ['name', 'reference']; public function __construct(array $attributes = []) { diff --git a/server/src/App/Models/Park.php b/server/src/App/Models/Park.php index b2802d27d..fcedda122 100644 --- a/server/src/App/Models/Park.php +++ b/server/src/App/Models/Park.php @@ -4,20 +4,13 @@ namespace Robert2\API\Models; use Illuminate\Database\Eloquent\SoftDeletes; -use Respect\Validation\Validator as V; +use Robert2\API\Validation\Validator as V; class Park extends BaseModel { use SoftDeletes; - protected $table = 'parks'; - - protected $_modelName = 'Park'; - protected $_orderField = 'name'; - protected $_orderDirection = 'asc'; - - protected $_allowedSearchFields = ['name']; - protected $_searchField = 'name'; + protected $searchField = 'name'; public function __construct(array $attributes = []) { diff --git a/server/src/App/Models/Person.php b/server/src/App/Models/Person.php index 283ff1f70..1fe230e19 100644 --- a/server/src/App/Models/Person.php +++ b/server/src/App/Models/Person.php @@ -7,10 +7,10 @@ use Illuminate\Database\Eloquent\Builder; use Illuminate\Database\Eloquent\SoftDeletes; use Illuminate\Database\QueryException; -use Respect\Validation\Validator as V; - -use Robert2\API\Errors; +use Robert2\API\Config\Config; use Robert2\API\Models\Traits\Taggable; +use Robert2\API\Validation\Validator as V; +use Robert2\API\Errors; class Person extends BaseModel { @@ -19,12 +19,10 @@ class Person extends BaseModel protected $table = 'persons'; - protected $_modelName = 'Person'; - protected $_orderField = 'first_name'; - protected $_orderDirection = 'asc'; + protected $orderField = 'first_name'; - protected $_allowedSearchFields = ['full_name', 'first_name', 'last_name', 'nickname', 'email']; - protected $_searchField = 'full_name'; + protected $allowedSearchFields = ['full_name', 'first_name', 'last_name', 'nickname', 'email']; + protected $searchField = 'full_name'; public function __construct(array $attributes = []) { @@ -131,11 +129,11 @@ public function getTagsAttribute() protected function _getOrderBy(?Builder $builder = null): Builder { - $order = $this->_orderField ?: 'id'; + $order = $this->orderField ?: 'id'; if ($order === 'company') { $order = 'companies.legal_name'; } - $direction = $this->_orderDirection ?: 'asc'; + $direction = $this->orderDirection ?: 'asc'; if ($builder) { $builder = $builder->orderBy($order, $direction); @@ -173,24 +171,22 @@ protected function _getOrderBy(?Builder $builder = null): Builder public function edit(?int $id = null, array $data = []): Model { - if (!empty($data['phone'])) { - $data['phone'] = normalizePhone($data['phone']); - } - if ($id && !$this->exists($id)) { - throw new Errors\NotFoundException("Edit model $this->_modelName failed, entity not found."); + throw new Errors\NotFoundException(sprintf("Edit failed, entity %d not found.", $id)); } $data = cleanEmptyFields($data); - $onlyFields = $id ? array_keys($data) : []; - $this->validate($data, $onlyFields); + if (!empty($data['phone'])) { + $data['phone'] = normalizePhone($data['phone']); + } try { - $person = self::updateOrCreate(['id' => $id], $data); + $person = self::firstOrNew(compact('id')); + $person->fill($data)->validate()->save(); if (!empty($data['tags'])) { - $this->setTags($person['id'], $data['tags']); + $this->setTags($person->id, $data['tags']); } } catch (QueryException $e) { if (!isDuplicateException($e)) { @@ -208,7 +204,7 @@ public function edit(?int $id = null, array $data = []): Model protected function _setOtherTag(Model $person): void { - $defaultTags = array_values($this->_settings['defaultTags']); + $defaultTags = array_values(Config::getSettings('defaultTags')); $existingTags = array_map(function ($tag) { return $tag['name']; }, $person->tags); @@ -223,13 +219,13 @@ protected function _setOtherTag(Model $person): void protected function _setSearchConditions(Builder $builder): Builder { - if (!$this->_searchField || !$this->_searchTerm) { + if (!$this->searchField || !$this->searchTerm) { return $builder; } - $term = sprintf('%%%s%%', addcslashes($this->_searchTerm, '%_')); + $term = sprintf('%%%s%%', addcslashes($this->searchTerm, '%_')); - if ($this->_searchField === 'full_name') { + if ($this->searchField === 'full_name') { $group = function (Builder $query) use ($term) { $query->orWhere('first_name', 'like', $term) ->orWhere('last_name', 'like', $term); @@ -237,6 +233,6 @@ protected function _setSearchConditions(Builder $builder): Builder return $builder->where($group); } - return $builder->where($this->_searchField, 'like', $term); + return $builder->where($this->searchField, 'like', $term); } } diff --git a/server/src/App/Models/SubCategory.php b/server/src/App/Models/SubCategory.php index 4bf2d84f4..b8dca7d37 100644 --- a/server/src/App/Models/SubCategory.php +++ b/server/src/App/Models/SubCategory.php @@ -4,20 +4,13 @@ namespace Robert2\API\Models; use Illuminate\Database\Eloquent\SoftDeletes; -use Respect\Validation\Validator as V; +use Robert2\API\Validation\Validator as V; class SubCategory extends BaseModel { use SoftDeletes; - protected $table = 'sub_categories'; - - protected $_modelName = 'SubCategory'; - protected $_orderField = 'name'; - protected $_orderDirection = 'asc'; - - protected $_allowedSearchFields = ['name']; - protected $_searchField = 'name'; + protected $searchField = 'name'; public function __construct(array $attributes = []) { diff --git a/server/src/App/Models/Tag.php b/server/src/App/Models/Tag.php index 3dbd057e5..b6c7bcc93 100644 --- a/server/src/App/Models/Tag.php +++ b/server/src/App/Models/Tag.php @@ -4,21 +4,16 @@ namespace Robert2\API\Models; use Illuminate\Database\Eloquent\SoftDeletes; -use Respect\Validation\Validator as V; use Illuminate\Database\Eloquent\Collection; +use Illuminate\Database\QueryException; +use Robert2\API\Validation\Validator as V; +use Robert2\API\Errors\ValidationException; class Tag extends BaseModel { use SoftDeletes; - protected $table = 'tags'; - - protected $_modelName = 'Tag'; - protected $_orderField = 'name'; - protected $_orderDirection = 'asc'; - - protected $_allowedSearchFields = ['name']; - protected $_searchField = 'name'; + protected $searchField = 'name'; public function __construct(array $attributes = []) { @@ -105,26 +100,36 @@ public function getIdsByNames(array $names): array public function bulkAdd(array $tagNames = []): array { - $tags = []; - $results = []; - foreach ($tagNames as $tagName) { - $existingTag = self::where('name', $tagName)->first(); - if ($existingTag) { - $results[] = $existingTag; - continue; - } - - $safeTag = ['name' => trim($tagName)]; - $this->validate($safeTag); - - $tags[] = $safeTag; - } + $tags = array_map( + function ($tagName) { + $existingTag = self::where('name', $tagName)->first(); + if ($existingTag) { + return $existingTag; + } + + $tag = new static(['name' => trim($tagName)]); + return tap($tag, function ($instance) { + $instance->validate(); + }); + }, + $tagNames + ); - foreach ($tags as $tagData) { - $results[] = self::edit(null, $tagData); - } + $this->getConnection()->transaction(function () use ($tags) { + try { + foreach ($tags as $tag) { + if (!$tag->exists || $tag->isDirty()) { + $tag->save(); + } + } + } catch (QueryException $e) { + $error = new ValidationException(); + $error->setPDOValidationException($e); + throw $error; + } + }); - return $results; + return $tags; } // —————————————————————————————————————————————————————— diff --git a/server/src/App/Models/Traits/Taggable.php b/server/src/App/Models/Traits/Taggable.php index 215e9a255..3163903f1 100644 --- a/server/src/App/Models/Traits/Taggable.php +++ b/server/src/App/Models/Traits/Taggable.php @@ -37,7 +37,7 @@ public function getAllFilteredOrTagged(array $conditions, array $tags = [], bool $builder = $builder->where($field, $value); } - if (!empty($this->_searchTerm)) { + if (!empty($this->searchTerm)) { $builder = $this->_setSearchConditions($builder); } diff --git a/server/src/App/Models/Traits/WithPdf.php b/server/src/App/Models/Traits/WithPdf.php index 2dec48f93..f0b70095a 100644 --- a/server/src/App/Models/Traits/WithPdf.php +++ b/server/src/App/Models/Traits/WithPdf.php @@ -22,7 +22,7 @@ public function getPdfName(int $id): string { $model = $this->withTrashed()->find($id); if (!$model) { - throw new NotFoundException(sprintf('%s not found.', $this->_modelName)); + throw new NotFoundException(sprintf('Record %d not found.', $id)); } $company = Config::getSettings('companyData'); @@ -30,7 +30,7 @@ public function getPdfName(int $id): string $i18n = new I18n(Config::getSettings('defaultLang')); $fileName = sprintf( '%s-%s-%s.pdf', - $i18n->translate($this->_modelName), + $i18n->translate(class_basename($this)), slugify($company['name']), $model->title ?: $model->id ); diff --git a/server/src/App/Models/User.php b/server/src/App/Models/User.php index 8b1345a11..d008af053 100644 --- a/server/src/App/Models/User.php +++ b/server/src/App/Models/User.php @@ -6,7 +6,7 @@ use Robert2\API\Config; use Illuminate\Database\Eloquent\Model; use Illuminate\Database\Eloquent\SoftDeletes; -use Respect\Validation\Validator as V; +use Robert2\API\Validation\Validator as V; use Illuminate\Database\Eloquent\Builder; use Robert2\API\Errors; @@ -15,14 +15,10 @@ class User extends BaseModel { use SoftDeletes; - protected $table = 'users'; + protected $orderField = 'pseudo'; - protected $_modelName = 'User'; - protected $_orderField = 'pseudo'; - protected $_orderDirection = 'asc'; - - protected $_allowedSearchFields = ['pseudo', 'email']; - protected $_searchField = 'pseudo'; + protected $allowedSearchFields = ['pseudo', 'email']; + protected $searchField = 'pseudo'; public function __construct(array $attributes = []) { diff --git a/server/src/App/Models/UserSetting.php b/server/src/App/Models/UserSetting.php index c21c85c4b..5bf3e0266 100644 --- a/server/src/App/Models/UserSetting.php +++ b/server/src/App/Models/UserSetting.php @@ -4,19 +4,13 @@ namespace Robert2\API\Models; use Illuminate\Database\Eloquent\Model; -use Respect\Validation\Validator as V; +use Robert2\API\Validation\Validator as V; use Illuminate\Database\Eloquent\Builder; use Robert2\API\Errors; class UserSetting extends BaseModel { - protected $table = 'user_settings'; - - protected $_modelName = 'UserSetting'; - protected $_orderField = 'id'; - protected $_orderDirection = 'asc'; - protected $dates = [ 'created_at', 'updated_at', diff --git a/server/src/App/Validation/Exceptions/CallbackException.php b/server/src/App/Validation/Exceptions/CallbackException.php new file mode 100644 index 000000000..c0f38b4cb --- /dev/null +++ b/server/src/App/Validation/Exceptions/CallbackException.php @@ -0,0 +1,9 @@ +callback = $callback; + $this->arguments = $arguments; + } + + public function validate($input) + { + $params = $this->arguments; + array_unshift($params, $input); + + $result = call_user_func_array($this->callback, $params); + if (is_bool($result)) { + return $result; + } + + $this->setTemplate($result); + return false; + } +} diff --git a/server/src/App/Validation/Validator.php b/server/src/App/Validation/Validator.php index 4dedc46be..411226fde 100644 --- a/server/src/App/Validation/Validator.php +++ b/server/src/App/Validation/Validator.php @@ -2,38 +2,37 @@ namespace Robert2\API\Validation; -use Respect\Validation\Exceptions\NestedValidationException; - use Robert2\API\I18n\I18n; +use Respect\Validation\Factory; +use Respect\Validation\Validator as CoreValidator; +use Respect\Validation\Exceptions\NestedValidationException; -class Validator +class Validator extends CoreValidator { - protected $_errors = []; - - public function validate(array $data, array $rules) + public function assert($input) { - foreach ($rules as $field => $rule) { - try { - $rule->setName($field)->assert(@$data[$field]); - } catch (NestedValidationException $e) { - $e->setParam('translator', [new I18n, 'translate']); - $this->addError($field, $e->getMessages()); - } + try { + parent::assert($input); + } catch (NestedValidationException $e) { + $e->setParam('translator', [new I18n, 'translate']); + throw $e; } - } - public function addError(string $field, array $message): void - { - $this->_errors[$field] = $message; + return true; } - public function hasError(): bool - { - return count($this->_errors) > 0; - } + // ------------------------------------------------------ + // - + // - Static methods + // - + // ------------------------------------------------------ - public function getErrors(): array + protected static function getFactory() { - return $this->_errors; + if (!static::$factory instanceof Factory) { + static::$factory = new Factory(); + static::$factory->prependRulePrefix('Robert2\\API\\Validation\\Rules'); + } + return static::$factory; } } diff --git a/server/tests/models/AttributeTest.php b/server/tests/models/AttributeTest.php index d1cd012f5..1b2453467 100644 --- a/server/tests/models/AttributeTest.php +++ b/server/tests/models/AttributeTest.php @@ -14,6 +14,11 @@ public function setup(): void $this->model = new Models\Attribute(); } + public function testTableName(): void + { + $this->assertEquals('attributes', $this->model->getTable()); + } + public function testGetAll(): void { $result = $this->model->getAll()->get()->toArray(); diff --git a/server/tests/models/BillTest.php b/server/tests/models/BillTest.php index e3c67ddc5..f0125e9cf 100644 --- a/server/tests/models/BillTest.php +++ b/server/tests/models/BillTest.php @@ -17,6 +17,11 @@ public function setup(): void $this->model = new Models\Bill(); } + public function testTableName(): void + { + $this->assertEquals('bills', $this->model->getTable()); + } + public function testGetAll(): void { $result = $this->model->getAll()->get()->toArray(); diff --git a/server/tests/models/CategoryTest.php b/server/tests/models/CategoryTest.php index a4523ef4c..583850623 100644 --- a/server/tests/models/CategoryTest.php +++ b/server/tests/models/CategoryTest.php @@ -14,6 +14,11 @@ public function setup(): void $this->model = new Models\Category(); } + public function testTableName(): void + { + $this->assertEquals('categories', $this->model->getTable()); + } + public function testGetAll(): void { $result = $this->model->getAll()->get()->toArray(); @@ -129,9 +134,10 @@ public function testBulkAdd(): void $this->assertEquals('one', @$result[0]['name']); $this->assertEquals('dès', @$result[1]['name']); - // - Ajout d'une catégorie, mais ignore la catégorie qui existe déjà - $result = $this->model->bulkAdd(['new categ', 'sound']); - $this->assertEquals('new categ', @$result[0]['name']); - $this->assertEmpty(@$result[1]['name']); + // - Ajout de catégories avec une qui existait déjà (ne l'ajoute pas deux fois) + $result = $this->model->bulkAdd(['new categorie', 'sound']); + $this->assertEquals('new categorie', @$result[0]['name']); + $this->assertEquals('sound', @$result[1]['name']); + $this->assertEquals(1, @$result[1]['id']); } } diff --git a/server/tests/models/CompanyTest.php b/server/tests/models/CompanyTest.php index f7d16c331..178e4f7a3 100644 --- a/server/tests/models/CompanyTest.php +++ b/server/tests/models/CompanyTest.php @@ -15,6 +15,11 @@ public function setup(): void $this->model = new Models\Company(); } + public function testTableName(): void + { + $this->assertEquals('companies', $this->model->getTable()); + } + public function testGetAll(): void { $results = $this->model->getAll()->get()->toArray(); diff --git a/server/tests/models/CountryTest.php b/server/tests/models/CountryTest.php index 718f2fe3c..d607be10c 100644 --- a/server/tests/models/CountryTest.php +++ b/server/tests/models/CountryTest.php @@ -14,6 +14,11 @@ public function setup(): void $this->model = new Models\Country(); } + public function testTableName(): void + { + $this->assertEquals('countries', $this->model->getTable()); + } + public function testGetAll(): void { $result = $this->model->getAll()->get()->toArray(); diff --git a/server/tests/models/EventTest.php b/server/tests/models/EventTest.php index 0818a3550..249fa91b3 100644 --- a/server/tests/models/EventTest.php +++ b/server/tests/models/EventTest.php @@ -15,6 +15,11 @@ public function setup(): void $this->model = new Models\Event(); } + public function testTableName(): void + { + $this->assertEquals('events', $this->model->getTable()); + } + public function testGetAll(): void { $this->model->setPeriod('2018-01-15', '2018-12-19'); @@ -267,7 +272,7 @@ public function testValidate(): void $data, ['end_date' => '2020-03-03 23:59:59'] ); - $this->model->validate($testData); + (new Models\Event($testData))->validate(); // - Validation fail: end date is after start date $this->expectException(Errors\ValidationException::class); @@ -276,7 +281,7 @@ public function testValidate(): void $data, ['end_date' => '2020-02-20 23:59:59'] ); - $this->model->validate($testData); + (new Models\Event($testData))->validate(); } public function testValidateReference(): void @@ -291,14 +296,14 @@ public function testValidateReference(): void foreach (['REF1', null] as $testValue) { $testData = array_merge($data, ['reference' => $testValue]); - $this->model->validate($testData); + (new Models\Event($testData))->validate(); } // - Validation fail: Reference is an empty string $this->expectException(Errors\ValidationException::class); $this->expectExceptionCode(ERROR_VALIDATION); $testData = array_merge($data, ['reference' => '']); - $this->model->validate($testData); + (new Models\Event($testData))->validate(); } public function testGetPdfContent() diff --git a/server/tests/models/MaterialTest.php b/server/tests/models/MaterialTest.php index 67915e236..c68c105c2 100644 --- a/server/tests/models/MaterialTest.php +++ b/server/tests/models/MaterialTest.php @@ -16,6 +16,11 @@ public function setup(): void $this->model = new Models\Material(); } + public function testTableName(): void + { + $this->assertEquals('materials', $this->model->getTable()); + } + public function testGetAll(): void { $result = $this->model->getAll()->get()->toArray(); diff --git a/server/tests/models/ParkTest.php b/server/tests/models/ParkTest.php index 73ec6ed0f..f4fc1b990 100644 --- a/server/tests/models/ParkTest.php +++ b/server/tests/models/ParkTest.php @@ -14,6 +14,11 @@ public function setup(): void $this->model = new Models\Park(); } + public function testTableName(): void + { + $this->assertEquals('parks', $this->model->getTable()); + } + public function testGetAll(): void { $result = $this->model->getAll()->get()->toArray(); diff --git a/server/tests/models/PersonTest.php b/server/tests/models/PersonTest.php index 381ca5409..19cfe66ef 100644 --- a/server/tests/models/PersonTest.php +++ b/server/tests/models/PersonTest.php @@ -15,6 +15,11 @@ public function setup(): void $this->model = new Models\Person(); } + public function testTableName(): void + { + $this->assertEquals('persons', $this->model->getTable()); + } + public function testGetAll(): void { $result = $this->model->getAll()->get()->toArray(); diff --git a/server/tests/models/SubCategoryTest.php b/server/tests/models/SubCategoryTest.php index 024fcfe5a..b052f03be 100644 --- a/server/tests/models/SubCategoryTest.php +++ b/server/tests/models/SubCategoryTest.php @@ -15,6 +15,11 @@ public function setup(): void $this->model = new Models\SubCategory(); } + public function testTableName(): void + { + $this->assertEquals('sub_categories', $this->model->getTable()); + } + public function testGetAll(): void { $result = $this->model->getAll()->get()->toArray(); diff --git a/server/tests/models/TagTest.php b/server/tests/models/TagTest.php index 693a5be57..b56a3b18d 100644 --- a/server/tests/models/TagTest.php +++ b/server/tests/models/TagTest.php @@ -15,6 +15,11 @@ public function setup(): void $this->model = new Models\Tag(); } + public function testTableName(): void + { + $this->assertEquals('tags', $this->model->getTable()); + } + public function testGetAll(): void { $result = $this->model->getAll()->get()->toArray(); @@ -101,8 +106,9 @@ public function testBulkAdd(): void // - Ajout de tags avec un qui existait déjà (ne l'ajoute pas deux fois) $result = $this->model->bulkAdd(['super tag', 'tag 01']); - $this->assertEquals('tag 01', @$result[0]['name']); - $this->assertEquals('super tag', @$result[1]['name']); + $this->assertEquals('super tag', @$result[0]['name']); + $this->assertEquals('tag 01', @$result[1]['name']); + $this->assertEquals(1, @$result[1]['id']); } public function testFormat(): void diff --git a/server/tests/models/UserSettingTest.php b/server/tests/models/UserSettingTest.php index 59a23242a..ab7216560 100644 --- a/server/tests/models/UserSettingTest.php +++ b/server/tests/models/UserSettingTest.php @@ -15,6 +15,11 @@ public function setup(): void $this->model = new Models\UserSetting(); } + public function testTableName(): void + { + $this->assertEquals('user_settings', $this->model->getTable()); + } + public function testGetAll(): void { $this->expectException(\Exception::class); diff --git a/server/tests/models/UserTest.php b/server/tests/models/UserTest.php index f81f00cf0..e8d3157f4 100644 --- a/server/tests/models/UserTest.php +++ b/server/tests/models/UserTest.php @@ -15,6 +15,11 @@ public function setup(): void $this->model = new Models\User(); } + public function testTableName(): void + { + $this->assertEquals('users', $this->model->getTable()); + } + private $expectedDataUser1 = [ 'id' => 1, 'pseudo' => 'test1',