From 59c823dd496bbc3fc6d2cc40284c5f81f4db53d1 Mon Sep 17 00:00:00 2001 From: Johan Cwiklinski Date: Sun, 27 Sep 2026 18:12:14 +0200 Subject: [PATCH 1/3] Inject login in plugin instead of reading a global --- lib/GaletteActivities/PluginGaletteActivities.php | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/lib/GaletteActivities/PluginGaletteActivities.php b/lib/GaletteActivities/PluginGaletteActivities.php index 6792189..073e993 100644 --- a/lib/GaletteActivities/PluginGaletteActivities.php +++ b/lib/GaletteActivities/PluginGaletteActivities.php @@ -34,6 +34,9 @@ class PluginGaletteActivities extends GalettePlugin implements MenuProviderInter #[Inject] private readonly Db $zdb; //@phpstan-ignore property.uninitializedReadonly,property.onlyRead (injected from DI) + #[Inject] + private readonly Login $login; //@phpstan-ignore property.uninitializedReadonly,property.onlyRead (injected from DI) + /** * Get plugins menus * @@ -41,11 +44,9 @@ class PluginGaletteActivities extends GalettePlugin implements MenuProviderInter */ public function getMenus(): array { - /** @var Login $login */ - global $login; $menus = []; - if ($login->isAdmin() || $login->isStaff()) { + if ($this->login->isAdmin() || $this->login->isStaff()) { $menus['plugin_activities'] = [ 'title' => _T("Activities", "activities"), 'icon' => 'calendar alternate', @@ -90,10 +91,7 @@ public function getPublicMenus(): array */ public function getListActions(Adherent $member): array { - /** @var Login $login */ - global $login; - - if (!$login->isAdmin() && !$login->isStaff()) { + if (!$this->login->isAdmin() && !$this->login->isStaff()) { return []; } From 8a481ac35f85476d42da3564bfc7675a50891a16 Mon Sep 17 00:00:00 2001 From: Johan Cwiklinski Date: Sun, 27 Sep 2026 18:13:14 +0200 Subject: [PATCH 2/3] Share entities, lists and controllers code; entities throw and keep their relations, lists load them once, sessions keep posted values --- .../Controllers/Crud/AbstractController.php | 233 ++++++++++++ .../Controllers/Crud/ActivitiesController.php | 239 +++++------- .../Crud/SubscriptionsController.php | 353 +++++++----------- lib/GaletteActivities/Entity/Activity.php | 206 ++++------ lib/GaletteActivities/Entity/EntityTrait.php | 120 ++++++ lib/GaletteActivities/Entity/Subscription.php | 303 +++++---------- lib/GaletteActivities/NotFoundException.php | 20 + .../Repository/AbstractRepository.php | 95 +++++ .../Repository/Activities.php | 133 ++----- .../Repository/Subscriptions.php | 341 ++++++----------- templates/default/activities.html.twig | 2 +- templates/default/activity.html.twig | 5 +- templates/default/subscription.html.twig | 6 +- templates/default/subscriptions.html.twig | 4 +- tests/ActivitiesFixtures.php | 15 + .../Crud/tests/units/ActivitiesController.php | 13 +- .../tests/units/SubscriptionsController.php | 21 +- .../Entity/tests/units/Activity.php | 43 +-- .../Entity/tests/units/Subscription.php | 85 +++-- .../Repository/tests/units/Activities.php | 2 +- .../Repository/tests/units/Subscriptions.php | 31 +- 21 files changed, 1150 insertions(+), 1120 deletions(-) create mode 100644 lib/GaletteActivities/Controllers/Crud/AbstractController.php create mode 100644 lib/GaletteActivities/Entity/EntityTrait.php create mode 100644 lib/GaletteActivities/NotFoundException.php create mode 100644 lib/GaletteActivities/Repository/AbstractRepository.php diff --git a/lib/GaletteActivities/Controllers/Crud/AbstractController.php b/lib/GaletteActivities/Controllers/Crud/AbstractController.php new file mode 100644 index 0000000..40429f3 --- /dev/null +++ b/lib/GaletteActivities/Controllers/Crud/AbstractController.php @@ -0,0 +1,233 @@ + + * + * @template TFilters of Pagination + */ +abstract class AbstractController extends AbstractPluginController +{ + /** + * @var array + */ + #[Inject("Plugin Galette Activities")] + protected array $module_info; + + /** + * Get default filter name, session key of list filters + */ + abstract public static function getDefaultFilterName(): string; + + /** + * Entity name, for session keys and logs: activity or subscription + */ + abstract protected function getEntityName(): string; + + /** + * Create empty list filters + * + * @return TFilters + */ + abstract protected function createFilters(): Pagination; + + /** + * Get the message for an entity that does not exist + * + * @param int $id Requested identifier + */ + abstract protected function getNotFoundMessage(int $id): string; + + /** + * Apply posted filters specific to the list + * + * @param TFilters $filters Filters + * @param array $post Posted values + */ + protected function applyPostedFilters(Pagination $filters, array $post): void + { + } + + /** + * Get list filters from session, with page or order of the list route + * + * @param string|null $option One of 'page' or 'order' + * @param string|int|null $value Value of the option + * + * @return TFilters + */ + protected function getFilters(?string $option = null, string|int|null $value = null): Pagination + { + $filters = $this->session->{$this->getFilterName($this->getDefaultFilterName())} ?? $this->createFilters(); + + switch ($option) { + case 'page': + $filters->current_page = (int)$value; + break; + case 'order': + $filters->orderby = $value; + break; + } + + return $filters; + } + + /** + * Store list filters in session + * + * @param Pagination $filters Filters + */ + protected function storeFilters(Pagination $filters): void + { + $this->session->{$this->getFilterName($this->getDefaultFilterName())} = $filters; + } + + /** + * Filtering + */ + public function filter(Request $request, Response $response): Response + { + $post = $request->getParsedBody(); + $filters = $this->getFilters(); + + if (isset($post['clear_filter'])) { + $filters->reinit(); + } else { + //number of rows to show + if (isset($post['nbshow'])) { + $filters->show = $post['nbshow']; + } + $this->applyPostedFilters($filters, $post); + } + + $this->storeFilters($filters); + return $this->redirect($response, $this->redirectUri([])); + } + + /** + * Keep posted values, to fill the form again after a redirection + * + * @param ?int $id Entity identifier, null for a new one + * @param array $post Posted values + */ + protected function keepPostedValues(?int $id, array $post): void + { + $this->session->{$this->getPostedValuesKey()} = [ + 'id' => $id, + 'values' => $post + ]; + } + + /** + * Get values posted on the form of an entity, once + * + * @param ?int $id Entity identifier, null for a new one + * + * @return ?array + */ + protected function getPostedValues(?int $id): ?array + { + $key = $this->getPostedValuesKey(); + $data = $this->session->$key ?? null; + unset($this->session->$key); + return is_array($data) && $data['id'] === $id ? $data['values'] : null; + } + + /** + * Session key of posted values + */ + private function getPostedValuesKey(): string + { + return 'plugin_activities_' . $this->getEntityName(); + } + + /** + * Store an entity, and report how it went + * + * @param Activity|Subscription $entity Entity + * @param string $added Message for a new entity + * @param string $modified Message for an existing entity + * @param string $failed Message when storage failed + * @param array $successes Success messages + * @param array $errors Error messages + */ + protected function storeEntity( + Activity|Subscription $entity, + string $added, + string $modified, + string $failed, + array &$successes, + array &$errors + ): void { + $new = $entity->getId() === null; + try { + $entity->store(); + $successes[] = $new ? $added : $modified; + } catch (\Throwable $e) { + Analog::log( + sprintf( + 'Unable to store %1$s #%2$s | %3$s', + $this->getEntityName(), + $entity->getId() ?? 'new', + $e->getMessage() + ), + Analog::ERROR + ); + $errors[] = $failed; + } + } + + /** + * Redirect when requested entity does not exist + * + * @param int $id Requested identifier + */ + protected function redirectNotFound(Response $response, int $id): Response + { + return $this->redirectWithErrors( + response: $response, + errors: [$this->getNotFoundMessage($id)], + redirect_url: $this->redirectUri([]) + )->withStatus(302); + } + + /** + * Report messages, and redirect after a POST: the browser follows with a GET + * + * @param Response $response PSR Response + * @param string $redirect_url URL to redirect to + * @param string[] $successes Successes to report + * @param string[] $warnings Warnings to report + * @param string[] $errors Errors to report + */ + protected function redirect( + Response $response, + string $redirect_url, + array $successes = [], + array $warnings = [], + array $errors = [] + ): Response { + return parent::redirect($response, $redirect_url, $successes, $warnings, $errors) + ->withStatus(303); + } +} diff --git a/lib/GaletteActivities/Controllers/Crud/ActivitiesController.php b/lib/GaletteActivities/Controllers/Crud/ActivitiesController.php index d6be5a1..2806285 100644 --- a/lib/GaletteActivities/Controllers/Crud/ActivitiesController.php +++ b/lib/GaletteActivities/Controllers/Crud/ActivitiesController.php @@ -10,29 +10,54 @@ namespace GaletteActivities\Controllers\Crud; -use Galette\Controllers\Crud\AbstractPluginController; +use Galette\Core\Pagination; use Galette\Repository\Groups; use GaletteActivities\Filters\ActivitiesList; use GaletteActivities\Entity\Activity; -use GaletteActivities\Entity\Subscription; +use GaletteActivities\NotFoundException; use GaletteActivities\Repository\Activities; use Slim\Psr7\Request; use Slim\Psr7\Response; -use DI\Attribute\Inject; /** * Activities controller * * @author Johan Cwiklinski + * + * @extends AbstractController */ -class ActivitiesController extends AbstractPluginController +class ActivitiesController extends AbstractController { /** - * @var array + * Entity name, for session keys and logs + */ + protected function getEntityName(): string + { + return 'activity'; + } + + /** + * Create empty list filters */ - #[Inject("Plugin Galette Activities")] - protected array $module_info; + protected function createFilters(): Pagination + { + return new ActivitiesList(); + } + + /** + * Get the message for an activity that does not exist + * + * @param int $id Requested activity identifier + */ + protected function getNotFoundMessage(int $id): string + { + return sprintf( + //TRANS: %1$s is the activity ID + _T('No activity #%1$s.', 'activities'), + $id + ); + } // CRUD - Create @@ -63,31 +88,13 @@ public function doAdd(Request $request, Response $response): Response */ public function list(Request $request, Response $response, ?string $option = null, string|int|null $value = null): Response { - $filter_name = $this->getFilterName($this->getDefaultFilterName()); - if (isset($this->session->$filter_name)) { - $filters = $this->session->$filter_name; - } else { - $filters = new ActivitiesList(); - } - - if ($option !== null) { - switch ($option) { - case 'page': - $filters->current_page = (int)$value; - break; - case 'order': - $filters->orderby = $value; - break; - } - } - - $activities = new Activities($this->zdb, $this->login, $this->preferences, $filters); + $filters = $this->getFilters($option, $value); + $activities = new Activities($this->zdb, $this->login, $this->history, $this->preferences, $filters); $list = $activities->getList(); //assign pagination variables to the template and add pagination links $filters->setViewPagination($this->routeparser, $this->view, false); - - $this->session->$filter_name = $filters; + $this->storeFilters($filters); // display page $this->view->render( @@ -104,36 +111,6 @@ public function list(Request $request, Response $response, ?string $option = nul return $response; } - /** - * Filtering - */ - public function filter(Request $request, Response $response): Response - { - $post = $request->getParsedBody(); - $filter_name = $this->getFilterName($this->getDefaultFilterName()); - if (isset($this->session->$filter_name)) { - $filters = $this->session->$filter_name; - } else { - $filters = new ActivitiesList(); - } - - //reinitialize filters - if (isset($post['clear_filter'])) { - $filters->reinit(); - } else { - //number of rows to show - if (isset($post['nbshow'])) { - $filters->show = $post['nbshow']; - } - } - - $this->session->$filter_name = $filters; - - return $response - ->withStatus(303) - ->withHeader('Location', $this->routeparser->urlFor('activities_activities')); - } - // /CRUD - Read // CRUD - Update @@ -145,30 +122,25 @@ public function filter(Request $request, Response $response): Response */ public function edit(Request $request, Response $response, ?int $id = null, string $action = 'edit'): Response { - $activity = $this->session->plugin_activities_activity ?? null; - if ($activity !== null) { - unset($this->session->plugin_activities_activity); - } else { - $activity = new Activity($this->zdb); + $activity = new Activity($this->zdb, $this->history); + + if ($id !== null) { + try { + $activity->load($id); + } catch (NotFoundException) { + return $this->redirectNotFound($response, $id); + } } - if ($id !== null && $activity->getId() != $id && !$activity->load($id)) { - $this->flash->addMessage( - 'error_detected', - sprintf( - //TRANS: %1$s is the activity ID - _T('No activity #%1$s.', 'activities'), - $id - ) - ); - return $response - ->withStatus(302) - ->withHeader('Location', $this->routeparser->urlFor('activities_activities')); + //values posted before an error + $values = $this->getPostedValues($activity->getId()); + if ($values !== null) { + $activity->check($values); } // template variable declaration $title = _T("Activity", "activities"); - if ($activity->getId() != '') { + if ($activity->getId() !== null) { $title .= ' (' . _T("modification") . ')'; } else { $title .= ' (' . _T("creation") . ')'; @@ -203,83 +175,45 @@ public function edit(Request $request, Response $response, ?int $id = null, stri public function doEdit(Request $request, Response $response, ?int $id = null, string $action = 'edit'): Response { $post = $request->getParsedBody(); - $activity = new Activity($this->zdb); - if (isset($post['id']) && !empty($post['id'])) { - $activity->load((int)$post['id']); - } - - $success_detected = []; - $error_detected = []; - - // Validation - $valid = $activity->check($post); - if ($valid !== true) { - $error_detected = array_merge($error_detected, $activity->getErrors()); - } - - if (count($error_detected) == 0) { - //all goes well, we can proceed - - $new = false; - if ($activity->getId() == '') { - $new = true; - } + $activity = new Activity($this->zdb, $this->history); + if (!empty($post['id'])) { try { - $store = $activity->store(); - } catch (\Throwable) { - //already logged by the entity - $store = false; - } - if ($store === true) { - //member has been stored :) - if ($new) { - $success_detected[] = _T("New activity has been successfully added.", "activities"); - } else { - $success_detected[] = _T("Activity has been modified.", "activities"); - } - } else { - //something went wrong :'( - $error_detected[] = _T("An error occurred while storing the activity.", "activities"); - } - } - - if (count($error_detected) > 0) { - foreach ($error_detected as $error) { - $this->flash->addMessage( - 'error_detected', - $error - ); + $activity->load((int)$post['id']); + } catch (NotFoundException) { + return $this->redirectNotFound($response, (int)$post['id']); } } - if (count($success_detected) > 0) { - foreach ($success_detected as $success) { - $this->flash->addMessage( - 'success_detected', - $success - ); - } + $successes = []; + $errors = []; + if ($activity->check($post)) { + $this->storeEntity( + $activity, + _T("New activity has been successfully added.", "activities"), + _T("Activity has been modified.", "activities"), + _T("An error occurred while storing the activity.", "activities"), + $successes, + $errors + ); + } else { + $errors = $activity->getErrors(); } - if (count($error_detected) == 0) { + if (count($errors) === 0) { $redirect_url = $this->routeparser->urlFor('activities_activities'); } else { - //store entity in session - $this->session->plugin_activities_activity = $activity; - - if ($activity->getId()) { - $redirect_url = $this->routeparser->urlFor( - 'activities_activity_edit', - ['id' => (string)$activity->getId()] - ); - } else { - $redirect_url = $this->routeparser->urlFor('activities_activity_add'); - } + $this->keepPostedValues($activity->getId(), $post); + $redirect_url = $activity->getId() !== null + ? $this->routeparser->urlFor('activities_activity_edit', ['id' => (string)$activity->getId()]) + : $this->routeparser->urlFor('activities_activity_add'); } - return $response - ->withStatus(303) - ->withHeader('Location', $redirect_url); + return $this->redirect( + response: $response, + redirect_url: $redirect_url, + successes: $successes, + errors: $errors + ); } // /CRUD - Update @@ -315,7 +249,11 @@ public function formUri(array $args): string */ public function confirmRemoveTitle(array $args): string { - $activity = new Activity($this->zdb, (int)$args['id']); + try { + $activity = new Activity($this->zdb, $this->history, (int)$args['id']); + } catch (NotFoundException) { + return $this->getNotFoundMessage((int)$args['id']); + } return sprintf( //TRANS %1$s is activity name _T('Remove activity %1$s', 'activities'), @@ -332,9 +270,11 @@ protected function getconfirmDeleteParams(Request $request): array { $params = parent::getconfirmDeleteParams($request); - $select = $this->zdb->select(ACTIVITIES_PREFIX . Subscription::TABLE); - $select->where([Activity::PK => (int)$params['data']['id']]); - $count = $this->zdb->execute($select)->count(); + try { + $count = (new Activity($this->zdb, $this->history, (int)$params['data']['id']))->countSubscriptions(); + } catch (NotFoundException) { + $count = 0; + } if ($count > 0) { $params['message'] = sprintf( _Tn( @@ -359,8 +299,9 @@ protected function getconfirmDeleteParams(Request $request): array */ protected function doDelete(array $args, array $post): bool { - $activity = new Activity($this->zdb, (int)$args['id']); - return $activity->remove(); + $activity = new Activity($this->zdb, $this->history, (int)$args['id']); + $activity->remove(); + return true; } // /CRUD - Delete diff --git a/lib/GaletteActivities/Controllers/Crud/SubscriptionsController.php b/lib/GaletteActivities/Controllers/Crud/SubscriptionsController.php index 6dfcb72..a83b0cf 100644 --- a/lib/GaletteActivities/Controllers/Crud/SubscriptionsController.php +++ b/lib/GaletteActivities/Controllers/Crud/SubscriptionsController.php @@ -10,31 +10,57 @@ namespace GaletteActivities\Controllers\Crud; +use Galette\Core\Pagination; use Galette\Entity\Adherent; use Galette\Repository\Members; -use Galette\Controllers\Crud\AbstractPluginController; use GaletteActivities\Filters\SubscriptionsList; use GaletteActivities\Entity\Subscription; use GaletteActivities\Entity\Activity; +use GaletteActivities\NotFoundException; use GaletteActivities\Repository\Subscriptions; use GaletteActivities\Repository\Activities; use Slim\Psr7\Request; use Slim\Psr7\Response; -use DI\Attribute\Inject; /** * Subscriptions controller * * @author Johan Cwiklinski + * + * @extends AbstractController */ -class SubscriptionsController extends AbstractPluginController +class SubscriptionsController extends AbstractController { /** - * @var array + * Entity name, for session keys and logs */ - #[Inject("Plugin Galette Activities")] - protected array $module_info; + protected function getEntityName(): string + { + return 'subscription'; + } + + /** + * Create empty list filters + */ + protected function createFilters(): Pagination + { + return new SubscriptionsList(); + } + + /** + * Get the message for a subscription that does not exist + * + * @param int $id Requested subscription identifier + */ + protected function getNotFoundMessage(int $id): string + { + return sprintf( + //TRANS: %1$s is the subscription ID + _T('No subscription #%1$s.', 'activities'), + $id + ); + } // CRUD - Create @@ -67,34 +93,27 @@ public function doAdd(Request $request, Response $response): Response */ public function list(Request $request, Response $response, ?string $option = null, string|int|null $value = null): Response { - $filters = $this->session->{$this->getFilterName($this->getDefaultFilterName())} ?? new SubscriptionsList(); - - if ($option !== null) { - switch ($option) { - case 'page': - $filters->current_page = (int)$value; - break; - case 'order': - $filters->orderby = $value; - break; - } - } + $filters = $this->getFilters($option, $value); $activity = null; - if ($filters->activity_filter) { - $activity = new Activity($this->zdb, (int)$filters->activity_filter); + if ($filters->activity_filter !== null) { + try { + $activity = new Activity($this->zdb, $this->history, (int)$filters->activity_filter); + } catch (NotFoundException) { + $filters->activity_filter = null; + } } - $subscriptions = new Subscriptions($this->zdb, $filters); + $subscriptions = new Subscriptions($this->zdb, $this->login, $this->history, $this->preferences, $filters); - $activities = new Activities($this->zdb, $this->login, $this->preferences); + $activities = new Activities($this->zdb, $this->login, $this->history, $this->preferences); $list = $subscriptions->getList(); $count = $subscriptions->getCount(); //assign pagination variables to the template and add pagination links $filters->setViewPagination($this->routeparser, $this->view, false); - $this->session->{$this->getFilterName($this->getDefaultFilterName())} = $filters; + $this->storeFilters($filters); // members $m = new Members(); @@ -128,73 +147,32 @@ public function list(Request $request, Response $response, ?string $option = nul } /** - * Filtering + * Apply posted subscriptions filters + * + * @param SubscriptionsList $filters Filters + * @param array $post Posted values */ - public function filter(Request $request, Response $response): Response + protected function applyPostedFilters(Pagination $filters, array $post): void { - $post = $request->getParsedBody(); - if (isset($this->session->{$this->getFilterName($this->getDefaultFilterName())})) { - $filters = $this->session->{$this->getFilterName($this->getDefaultFilterName())}; - } else { - $filters = new SubscriptionsList(); - } - - //reinitialize filters - if (isset($post['clear_filter'])) { - $filters->reinit(); - } else { - //number of rows to show - if (isset($post['nbshow'])) { - $filters->show = $post['nbshow']; - } - - if (isset($post['paid_filter'])) { - if (is_numeric($post['paid_filter'])) { - $filters->paid_filter = $post['paid_filter']; - } - } - - if (isset($post['payment_type_filter'])) { - if (is_numeric($post['payment_type_filter'])) { - $filters->payment_type_filter = $post['payment_type_filter']; - } - } - - if (isset($post['activity_filter'])) { - if (is_numeric($post['activity_filter'])) { - $filters->activity_filter = $post['activity_filter']; - } - } - - if (isset($post['member_filter'])) { - if ($post['member_filter'] === '' || is_numeric($post['member_filter'])) { - $filters->member_filter = $post['member_filter']; - } - } - - if (isset($post['date_field'])) { - if (is_numeric($post['date_field'])) { - $filters->date_field = $post['date_field']; - } - } - - if (isset($post['start_date_filter'])) { - $filters->start_date_filter = $post['start_date_filter']; + foreach (['paid_filter', 'payment_type_filter', 'activity_filter', 'date_field'] as $name) { + if (isset($post[$name]) && is_numeric($post[$name])) { + $filters->$name = $post[$name]; } + } - if (isset($post['end_date_filter'])) { - $filters->end_date_filter = $post['end_date_filter']; + if (isset($post['member_filter'])) { + if ($post['member_filter'] === '' || is_numeric($post['member_filter'])) { + $filters->member_filter = $post['member_filter']; } } - $this->session->{$this->getFilterName($this->getDefaultFilterName())} = $filters; + if (isset($post['start_date_filter'])) { + $filters->start_date_filter = $post['start_date_filter']; + } - return $response - ->withStatus(303) - ->withHeader( - 'Location', - $this->routeparser->urlFor('activities_subscriptions') - ); + if (isset($post['end_date_filter'])) { + $filters->end_date_filter = $post['end_date_filter']; + } } // /CRUD - Read @@ -210,42 +188,34 @@ public function filter(Request $request, Response $response): Response public function edit(Request $request, Response $response, ?int $id = null, string $action = 'edit', ?int $id_adh = null): Response { $route_params = []; + $subscription = new Subscription($this->zdb, $this->history); - $subscription = $this->session->plugin_activities_subscription ?? null; - if ($subscription !== null) { - unset($this->session->plugin_activities_subscription); - } else { - $subscription = new Subscription($this->zdb); - } - - if ($id !== null && $subscription->getId() != $id) { - if (!$subscription->load($id)) { - $this->flash->addMessage( - 'error_detected', - sprintf( - //TRANS: %1$s is the subscription ID - _T('No subscription #%1$s.', 'activities'), - $id - ) - ); - return $response - ->withStatus(302) - ->withHeader('Location', $this->routeparser->urlFor('activities_subscriptions')); + if ($id !== null) { + try { + $subscription->load($id); + } catch (NotFoundException) { + return $this->redirectNotFound($response, $id); } } elseif ($id_adh !== null) { $subscription->setMember($id_adh); } + //values posted before an error, or to reload the form + $values = $this->getPostedValues($subscription->getId()); + if ($values !== null) { + $subscription->check($values); + } + // template variable declaration $title = _T("Subscription", "activities"); - if ($subscription->getId() != '') { + if ($subscription->getId() !== null) { $title .= ' (' . _T("modification") . ')'; } else { $title .= ' (' . _T("creation") . ')'; } //Activities - $activities = new Activities($this->zdb, $this->login, $this->preferences); + $activities = new Activities($this->zdb, $this->login, $this->history, $this->preferences); // members $m = new Members(); @@ -299,125 +269,77 @@ public function edit(Request $request, Response $response, ?int $id = null, stri public function doEdit(Request $request, Response $response, ?int $id = null, string $action = 'edit'): Response { $post = $request->getParsedBody(); - $subscription = new Subscription($this->zdb); - if (isset($post['id']) && !empty($post['id'])) { - $subscription->load((int)$post['id']); - } - - if (isset($post['cancel'])) { - $redirect_url = $this->routeparser->urlFor( - 'activities_subscriptions' - ); - return $response - ->withStatus(303) - ->withHeader('Location', $redirect_url); - } - - $success_detected = []; - $warning_detected = []; - $error_detected = []; - $goto_list = true; - - // Validation - $valid = $subscription->check($post); - if ($valid !== true) { - $error_detected = array_merge($error_detected, $subscription->getErrors()); - } - - if (count($error_detected) == 0 && isset($post['save'])) { - //all goes well, we can proceed - - $new = false; - if ($subscription->getId() == '') { - $new = true; - } + $subscription = new Subscription($this->zdb, $this->history); + if (!empty($post['id'])) { try { - $store = $subscription->store(); - } catch (\Throwable) { - //already logged by the entity - $store = false; - } - if ($store === true) { - //member has been stored :) - if ($new) { - $success_detected[] = _T("New subscription has been successfully added.", "activities"); - } else { - $success_detected[] = _T("Subscription has been modified.", "activities"); - } - } else { - //something went wrong :'( - $errors = $subscription->getErrors(); - if (count($errors)) { - $error_detected = array_merge($error_detected, $errors); - } else { - $error_detected[] = _T("An error occurred while storing the subscription.", "activities"); - } + $subscription->load((int)$post['id']); + } catch (NotFoundException) { + return $this->redirectNotFound($response, (int)$post['id']); } } - if (!isset($post['save'])) { - $this->session->plugin_activities_subscription = $subscription; - $error_detected = []; - $goto_list = false; - $warning_detected[] = _T('Do not forget to store the subscription', 'activities'); + if (isset($post['cancel'])) { + return $this->redirect($response, $this->routeparser->urlFor('activities_subscriptions')); } - if (count($error_detected) > 0) { - foreach ($error_detected as $error) { - $this->flash->addMessage( - 'error_detected', - $error - ); - } + //form posted to be reloaded, with values of the chosen activity + if (!isset($post['save'])) { + $this->keepPostedValues($subscription->getId(), $post); + return $this->redirect( + response: $response, + redirect_url: $this->getFormUrl($subscription), + warnings: [_T('Do not forget to store the subscription', 'activities')] + ); } - if (count($warning_detected) > 0) { - foreach ($warning_detected as $warning) { - $this->flash->addMessage( - 'warning_detected', - $warning - ); - } - } - if (count($success_detected) > 0) { - foreach ($success_detected as $success) { - $this->flash->addMessage( - 'success_detected', - $success - ); - } + $successes = []; + $errors = []; + if ($subscription->check($post)) { + $this->storeEntity( + $subscription, + _T("New subscription has been successfully added.", "activities"), + _T("Subscription has been modified.", "activities"), + _T("An error occurred while storing the subscription.", "activities"), + $successes, + $errors + ); + } else { + $errors = $subscription->getErrors(); } - if (count($error_detected) == 0 && $goto_list) { + if (count($errors) === 0) { //show subscriptions of the stored activity - $filter_name = $this->getFilterName($this->getDefaultFilterName()); - $filters = $this->session->$filter_name ?? new SubscriptionsList(); + $filters = $this->getFilters(); $filters->activity_filter = $subscription->getActivityId(); - $this->session->$filter_name = $filters; + $this->storeFilters($filters); $redirect_url = $this->routeparser->urlFor('activities_subscriptions'); } else { - //store entity in session - $this->session->plugin_activities_subscription = $subscription; - - if ($subscription->getId()) { - $route = 'activities_subscription_edit'; - $rparams = [ - 'id' => $subscription->getId(), - 'action' => 'edit' - ]; - } else { - $route = 'activities_subscription_add'; - $rparams = ['action' => 'add']; - } - $redirect_url = $this->routeparser->urlFor( - $route, - $rparams - ); + $this->keepPostedValues($subscription->getId(), $post); + $redirect_url = $this->getFormUrl($subscription); } - return $response - ->withStatus(303) - ->withHeader('Location', $redirect_url); + return $this->redirect( + response: $response, + redirect_url: $redirect_url, + successes: $successes, + errors: $errors + ); + } + + /** + * Get URL of the form of a subscription + * + * @param Subscription $subscription Subscription + */ + private function getFormUrl(Subscription $subscription): string + { + if ($subscription->getId() !== null) { + return $this->routeparser->urlFor( + 'activities_subscription_edit', + ['id' => (string)$subscription->getId()] + ); + } + return $this->routeparser->urlFor('activities_subscription_add'); } // /CRUD - Update @@ -453,14 +375,16 @@ public function formUri(array $args): string */ public function confirmRemoveTitle(array $args): string { - $subscription = new Subscription($this->zdb, (int)$args['id']); - $member = $subscription->getMember(); - $activity = $subscription->getActivity(); + try { + $subscription = new Subscription($this->zdb, $this->history, (int)$args['id']); + } catch (NotFoundException) { + return $this->getNotFoundMessage((int)$args['id']); + } return sprintf( //TRANS: %1$s is the member name, %2$s the activity name. _T('Remove subscription for %1$s on %2$s', 'activities'), - $member->sname, - $activity->getName() + $subscription->getMember()?->sname, + $subscription->getActivity()?->getName() ); } @@ -472,8 +396,9 @@ public function confirmRemoveTitle(array $args): string */ protected function doDelete(array $args, array $post): bool { - $subscription = new Subscription($this->zdb, (int)$args['id']); - return $subscription->remove(); + $subscription = new Subscription($this->zdb, $this->history, (int)$args['id']); + $subscription->remove(); + return true; } // /CRUD - Delete diff --git a/lib/GaletteActivities/Entity/Activity.php b/lib/GaletteActivities/Entity/Activity.php index 05fa55f..60d5b14 100644 --- a/lib/GaletteActivities/Entity/Activity.php +++ b/lib/GaletteActivities/Entity/Activity.php @@ -12,6 +12,7 @@ use ArrayObject; use Galette\Core\Db; +use Galette\Core\History; use Analog\Analog; use Galette\Entity\Group; use Galette\Helpers\EntityHelper; @@ -25,70 +26,47 @@ class Activity { use EntityHelper; + use EntityTrait; public const string TABLE = 'activities'; public const string PK = 'id_activity'; private Db $zdb; + private History $history; /** @var array */ private array $errors = []; - private int $id; - private string $name; - private string $type; + private ?int $id = null; + private string $name = ''; + private string $type = ''; private ?float $price = null; private ?int $id_group = null; private ?Group $group = null; private ?string $creation_date = null; - private ?string $comment; + private ?string $comment = null; /** * Default constructor * - * @param Db $zdb Database instance - * @param null|int|ArrayObject $args Either a ResultSet row or its id for to load - * a specific activity, or null to just - * instanciate object + * @param Db $zdb Database instance + * @param History $history History instance + * @param null|int|ArrayObject $args Either a ResultSet row or its id for to load + * a specific activity, or null to just + * instanciate object */ - public function __construct(Db $zdb, int|ArrayObject|null $args = null) + public function __construct(Db $zdb, History $history, int|ArrayObject|null $args = null) { $this->zdb = $zdb; + $this->history = $history; $this->setFields(); - if (is_int($args) && $args > 0) { + if (is_int($args)) { $this->load($args); - } elseif (is_object($args)) { + } elseif ($args !== null) { $this->loadFromRS($args); } } - /** - * Loads an activity from its id - * - * @param int $id the identifiant for the activity to load - */ - public function load(int $id): bool - { - try { - $select = $this->zdb->select($this->getTableName()); - $select->where([self::PK => $id]); - $results = $this->zdb->execute($select); - - if ($results->count() > 0) { - $this->loadFromRS($results->current()); - return true; - } else { - return false; - } - } catch (\Exception $e) { - Analog::log( - 'Cannot load activity #`' . $id . '` | ' . $e->getMessage(), - Analog::WARNING - ); - throw $e; - } - } - /** * Populate object from a resultset row * @@ -99,53 +77,12 @@ private function loadFromRS(ArrayObject $r): void $this->id = (int)$r['id_activity']; $this->name = $r['name']; $this->type = $r['type'] ?? ''; - if ($r['price'] !== null) { - $this->price = (float)$r['price']; - } - if ($r['id_group'] !== null) { - $this->id_group = (int)$r['id_group']; - $this->group = new Group($this->id_group); - } + $this->price = $r['price'] === null ? null : (float)$r['price']; + $this->id_group = $r['id_group'] === null ? null : (int)$r['id_group']; $this->creation_date = $r['creation_date']; $this->comment = $r['comment']; } - /** - * Remove specified activity - */ - public function remove(): bool - { - $transaction = false; - - try { - if (!$this->zdb->connection->inTransaction()) { - $this->zdb->connection->beginTransaction(); - $transaction = true; - } - - $delete = $this->zdb->delete($this->getTableName()); - $delete->where([self::PK => $this->id]); - $this->zdb->execute($delete); - - //commit all changes - if ($transaction) { - $this->zdb->connection->commit(); - } - - return true; - } catch (\Exception $e) { - if ($transaction) { - $this->zdb->connection->rollBack(); - } - Analog::log( - 'Unable to delete activity ' . $this->name - . ' (' . $this->id . ') |' . $e->getMessage(), - Analog::ERROR - ); - return false; - } - } - /** * Check posted values validity * @@ -186,10 +123,8 @@ public function check(array $values): bool if (isset($values['id_group']) && !empty($values['id_group'])) { $this->id_group = (int)$values['id_group']; - $this->group = new Group($this->id_group); } else { $this->id_group = null; - $this->group = null; } if (isset($values['comment']) && !empty($values['comment'])) { @@ -213,11 +148,9 @@ public function check(array $values): bool /** * Store the activity */ - public function store(): bool + public function store(): void { - global $hist; - - try { + $this->transactional(function (): void { $values = [ 'name' => $this->name, 'type' => $this->type, @@ -226,7 +159,7 @@ public function store(): bool 'comment' => $this->comment ?? new Expression('NULL') ]; - if (empty($this->id)) { + if ($this->id === null) { //we're inserting a new activity $this->creation_date = date("Y-m-d"); $values['creation_date'] = $this->creation_date; @@ -234,31 +167,21 @@ public function store(): bool $insert = $this->zdb->insert($this->getTableName()); $insert->values($values); $add = $this->zdb->execute($insert); - if ($add->count() > 0) { - if ($this->zdb->isPostgres()) { - /** @phpstan-ignore-next-line */ - $this->id = (int)$this->zdb->driver->getLastGeneratedValue( - PREFIX_DB . $this->getTableName() . '_id_seq' - ); - } else { - $this->id = (int)$this->zdb->driver->getLastGeneratedValue(); - } - - // logging - $hist->add( - _T("Activity added", "activities"), - $this->name - ); - return true; - } else { - $hist->add(_T("Fail to add new activity.", "activities")); - throw new \Exception( + if ($add->count() === 0) { + $this->history->add(_T("Fail to add new activity.", "activities")); + throw new \RuntimeException( 'An error occurred inserting new activity!' ); } + $this->id = $this->getLastInsertId(); + + // logging + $this->history->add( + _T("Activity added", "activities"), + $this->name + ); } else { //we're editing an existing activity - $values[self::PK] = $this->id; $update = $this->zdb->update($this->getTableName()); $update ->set($values) @@ -269,21 +192,28 @@ public function store(): bool //edit == 0 does not mean there were an error, but that there //were nothing to change if ($edit->count() > 0) { - $hist->add( + $this->history->add( _T("Activity updated", "activities"), $this->name ); } - return true; } - } catch (\Exception $e) { - Analog::log( - 'Something went wrong :\'( | ' . $e->getMessage() . "\n" - . $e->getTraceAsString(), - Analog::ERROR - ); - throw $e; + }); + } + + /** + * Count subscriptions to this activity + */ + public function countSubscriptions(): int + { + if ($this->id === null) { + return 0; } + + $select = $this->zdb->select(ACTIVITIES_PREFIX . Subscription::TABLE); + $select->columns(['counter' => new Expression('COUNT(' . Subscription::PK . ')')]) + ->where([self::PK => $this->id]); + return (int)$this->zdb->execute($select)->current()['counter']; } /** @@ -291,7 +221,7 @@ public function store(): bool */ public function getId(): ?int { - return $this->id ?? null; + return $this->id; } /** @@ -299,7 +229,7 @@ public function getId(): ?int */ public function getName(): string { - return $this->name ?? ''; + return $this->name; } /** @@ -307,17 +237,15 @@ public function getName(): string */ public function getType(): string { - return $this->type ?? ''; + return $this->type; } /** - * Get creation date - * - * @param bool $formatted Return date formatted, raw if false + * Get creation date, as Y-m-d */ - public function getCreationDate(bool $formatted = true): string + public function getCreationDate(): string { - return $this->getDate('creation_date', $formatted) ?? ''; + return $this->creation_date ?? ''; } /** @@ -329,19 +257,25 @@ public function getPrice(): ?float } /** - * Get Group + * Get group id */ - public function getGroup(): ?Group + public function getGroupId(): ?int { - return $this->group; + return $this->id_group; } /** - * Get table's name + * Get group, loaded once */ - protected function getTableName(): string + public function getGroup(): ?Group { - return ACTIVITIES_PREFIX . self::TABLE; + if ($this->id_group === null) { + return null; + } + if ($this->group?->getId() !== $this->id_group) { + $this->group = new Group($this->id_group); + } + return $this->group; } /** @@ -352,16 +286,6 @@ public function getComment(): string return $this->comment ?? ''; } - /** - * Get errors - * - * @return array - */ - public function getErrors(): array - { - return $this->errors; - } - /** * Set fields, must populate $this->fields */ diff --git a/lib/GaletteActivities/Entity/EntityTrait.php b/lib/GaletteActivities/Entity/EntityTrait.php new file mode 100644 index 0000000..8fa2d74 --- /dev/null +++ b/lib/GaletteActivities/Entity/EntityTrait.php @@ -0,0 +1,120 @@ + + */ +trait EntityTrait +{ + /** + * Populate object from a resultset row + * + * @param ArrayObject $r the resultset row + */ + abstract private function loadFromRS(ArrayObject $r): void; + + /** + * Load entity from its id + * + * @param int $id Identifier + * + * @throws NotFoundException + */ + public function load(int $id): void + { + $select = $this->zdb->select($this->getTableName()); + $select->where([self::PK => $id]); + $results = $this->zdb->execute($select); + + if ($results->count() === 0) { + throw new NotFoundException(sprintf('%1$s #%2$s does not exist', self::class, $id)); + } + $this->loadFromRS($results->current()); + } + + /** + * Remove entity; database removes its links + */ + public function remove(): void + { + $delete = $this->zdb->delete($this->getTableName()); + $delete->where([self::PK => $this->id]); + $this->zdb->execute($delete); + } + + /** + * Run storage in a transaction, unless one is already running + * + * @param callable $store Storage + */ + private function transactional(callable $store): void + { + $new = $this->id === null; + $transaction = !$this->zdb->connection->inTransaction(); + if ($transaction) { + $this->zdb->connection->beginTransaction(); + } + + try { + $store(); + if ($transaction) { + $this->zdb->connection->commit(); + } + } catch (\Throwable $e) { + if ($transaction) { + $this->zdb->connection->rollBack(); + } + if ($new) { + //nothing has been stored + $this->id = null; + } + throw $e; + } + } + + /** + * Get identifier of the row that has just been inserted + */ + private function getLastInsertId(): int + { + if ($this->zdb->isPostgres()) { + /** @phpstan-ignore-next-line */ + return (int)$this->zdb->driver->getLastGeneratedValue( + PREFIX_DB . $this->getTableName() . '_id_seq' + ); + } + return (int)$this->zdb->driver->getLastGeneratedValue(); + } + + /** + * Get table's name + */ + protected function getTableName(): string + { + return ACTIVITIES_PREFIX . self::TABLE; + } + + /** + * Get errors + * + * @return array + */ + public function getErrors(): array + { + return $this->errors; + } +} diff --git a/lib/GaletteActivities/Entity/Subscription.php b/lib/GaletteActivities/Entity/Subscription.php index b5dae60..4ab8a66 100644 --- a/lib/GaletteActivities/Entity/Subscription.php +++ b/lib/GaletteActivities/Entity/Subscription.php @@ -12,11 +12,13 @@ use ArrayObject; use Galette\Core\Db; +use Galette\Core\History; use Galette\Entity\Adherent; use Galette\Entity\Group; use Galette\Entity\PaymentType; use Analog\Analog; use Galette\Helpers\EntityHelper; +use GaletteActivities\NotFoundException; /** * Subscription entity @@ -26,18 +28,20 @@ class Subscription { use EntityHelper; + use EntityTrait; public const string TABLE = 'subscriptions'; public const string PK = 'id_subscription'; private Db $zdb; + private History $history; /** @var array */ - private array $errors; + private array $errors = []; - private int $id; - private int $id_activity; + private ?int $id = null; + private ?int $id_activity = null; private ?Activity $activity = null; - private int $id_member; + private ?int $id_member = null; private ?Adherent $member = null; //activity and member as stored in database private ?int $stored_activity = null; @@ -53,52 +57,26 @@ class Subscription /** * Default constructor * - * @param Db $zdb Database instance - * @param null|int|ArrayObject $args Either a ResultSet row or its id for to load - * a specific subscription, or null to just - * instanciate object + * @param Db $zdb Database instance + * @param History $history History instance + * @param null|int|ArrayObject $args Either a ResultSet row or its id for to load + * a specific subscription, or null to just + * instanciate object */ - public function __construct(Db $zdb, int|ArrayObject|null $args = null) + public function __construct(Db $zdb, History $history, int|ArrayObject|null $args = null) { $this->zdb = $zdb; + $this->history = $history; $this->setFields(); $this->creation_date = date("Y-m-d"); if (is_int($args)) { $this->load($args); - } elseif (is_object($args)) { + } elseif ($args !== null) { $this->loadFromRS($args); } } - /** - * Loads a subscription from its id - * - * @param int $id the identifiant for the subscription to load - */ - public function load(int $id): bool - { - try { - $select = $this->zdb->select($this->getTableName()); - $select->where([self::PK => $id]); - - $results = $this->zdb->execute($select); - - if ($results->count() > 0) { - $this->loadFromRS($results->current()); - return true; - } else { - return false; - } - } catch (\Exception $e) { - Analog::log( - 'Cannot load subscription form id `' . $id . '` | ' . $e->getMessage(), - Analog::WARNING - ); - throw $e; - } - } - /** * Populate object from a resultset row * @@ -107,14 +85,12 @@ public function load(int $id): bool private function loadFromRS(ArrayObject $r): void { $this->id = (int)$r['id_subscription']; - $this->setActivity((int)$r[Activity::PK]); - $this->setMember((int)$r[Adherent::PK]); + $this->id_activity = (int)$r[Activity::PK]; + $this->id_member = (int)$r[Adherent::PK]; $this->stored_activity = $this->id_activity; $this->stored_member = $this->id_member; $this->paid = (bool)$r['is_paid']; - if ($r['payment_amount'] !== null) { - $this->payment_amount = (float)$r['payment_amount']; - } + $this->payment_amount = $r['payment_amount'] === null ? null : (float)$r['payment_amount']; $this->payment_method = (int)$r['payment_method']; $this->creation_date = $r['creation_date']; $this->subscription_date = $r['subscription_date']; @@ -122,42 +98,6 @@ private function loadFromRS(ArrayObject $r): void $this->comment = $r['comment'] ?? ''; } - /** - * Remove specified subscription - */ - public function remove(): bool - { - $transaction = false; - - try { - if (!$this->zdb->connection->inTransaction()) { - $this->zdb->connection->beginTransaction(); - $transaction = true; - } - - $delete = $this->zdb->delete($this->getTableName()); - $delete->where([self::PK => $this->id]); - $this->zdb->execute($delete); - - //commit all changes - if ($transaction) { - $this->zdb->connection->commit(); - } - - return true; - } catch (\Exception $e) { - if ($transaction) { - $this->zdb->connection->rollBack(); - } - Analog::log( - 'Unable to delete subscription ' - . ' (' . $this->id . ') |' . $e->getMessage(), - Analog::ERROR - ); - return false; - } - } - /** * Check posted values validity * @@ -171,8 +111,9 @@ public function check(array $values): bool if (!isset($values['activity']) || empty($values['activity']) || $values['activity'] == -1) { $this->errors[] = _T('Activity is mandatory', 'activities'); } else { - $this->setActivity((int)$values['activity']); - if ($this->activity?->getId() === null) { + try { + $this->useActivity(new Activity($this->zdb, $this->history, (int)$values['activity'])); + } catch (NotFoundException) { $this->errors[] = sprintf( //TRANS: %1$s is the activity ID _T('No activity #%1$s.', 'activities'), @@ -199,10 +140,10 @@ public function check(array $values): bool } else { $this->errors[] = _T('Amount must be a number.', 'activities'); } - } elseif ($amount === null || empty($this->id)) { + } elseif ($amount === null || $this->id === null) { //new subscriptions default to activity price; existing ones can be cleared - if (isset($values['save']) && $this->activity !== null) { - $this->payment_amount = $this->activity->getPrice(); + if (isset($values['save']) && $this->getActivity() !== null) { + $this->payment_amount = $this->getActivity()->getPrice(); } } else { $this->payment_amount = null; @@ -251,6 +192,10 @@ public function check(array $values): bool $this->errors[] = _T('End date must not be before subscription date.', 'activities'); } + if (count($this->errors) === 0 && $this->isDuplicate()) { + $this->errors[] = _T('Subscription already exists for this member and activity', 'activities'); + } + if (count($this->errors) > 0) { Analog::log( 'Some errors has been threw attempting to edit/store a subscription' . "\n" @@ -278,22 +223,9 @@ private function memberExists(int $id): bool /** * Store the subscription */ - public function store(): bool + public function store(): void { - global $hist; - - if ($this->isDuplicate()) { - //checked before any query: on PostgreSQL, a failing query aborts the whole transaction - $this->errors[] = _T('Subscription already exists for this member and activity', 'activities'); - return false; - } - - $transaction = false; - try { - if (!$this->zdb->connection->inTransaction()) { - $this->zdb->connection->beginTransaction(); - $transaction = true; - } + $this->transactional(function (): void { $values = [ Activity::PK => $this->id_activity, Adherent::PK => $this->id_member, @@ -307,35 +239,26 @@ public function store(): bool 'comment' => $this->comment ]; - if (empty($this->id)) { + if ($this->id === null) { //we're inserting a new subscription $insert = $this->zdb->insert($this->getTableName()); $insert->values($values); $add = $this->zdb->execute($insert); - if ($add->count() > 0) { - if ($this->zdb->isPostgres()) { - /** @phpstan-ignore-next-line */ - $this->id = (int)$this->zdb->driver->getLastGeneratedValue( - PREFIX_DB . ACTIVITIES_PREFIX . self::TABLE . '_id_seq' - ); - } else { - $this->id = (int)$this->zdb->driver->getLastGeneratedValue(); - } - - // logging - $hist->add( - _T("Subscription added", "activities"), - $this->getActivity()->getName() - ); - } else { - $hist->add(_T("Fail to add new subscription.", "activities")); - throw new \Exception( + if ($add->count() === 0) { + $this->history->add(_T("Fail to add new subscription.", "activities")); + throw new \RuntimeException( 'An error occurred inserting new subscription!' ); } + $this->id = $this->getLastInsertId(); + + // logging + $this->history->add( + _T("Subscription added", "activities"), + $this->getActivity()?->getName() ?? '' + ); } else { //we're editing an existing subscription - $values[self::PK] = $this->id; $update = $this->zdb->update($this->getTableName()); $update ->set($values) @@ -346,7 +269,7 @@ public function store(): bool //edit == 0 does not mean there were an error, but that there //were nothing to change if ($edit->count() > 0) { - $hist->add( + $this->history->add( _T("Subscription updated", "activities") ); } @@ -356,30 +279,9 @@ public function store(): bool if ($this->id_activity !== $this->stored_activity || $this->id_member !== $this->stored_member) { $this->joinActivityGroup(); } - - if ($transaction) { - $this->zdb->connection->commit(); - } - $this->stored_activity = $this->id_activity; - $this->stored_member = $this->id_member; - return true; - } catch (\OverflowException $e) { - if ($transaction) { - $this->zdb->connection->rollBack(); - } - $this->errors[] = _T('Subscription already exists for this member and activity', 'activities'); - return false; - } catch (\Exception $e) { - if ($transaction) { - $this->zdb->connection->rollBack(); - } - Analog::log( - 'Something went wrong :\'( | ' . $e->getMessage() . "\n" - . $e->getTraceAsString(), - Analog::ERROR - ); - throw $e; - } + }); + $this->stored_activity = $this->id_activity; + $this->stored_member = $this->id_member; } /** @@ -392,7 +294,7 @@ private function isDuplicate(): bool Activity::PK => $this->id_activity, Adherent::PK => $this->id_member ]); - if (!empty($this->id)) { + if ($this->id !== null) { $select->where->notEqualTo(self::PK, $this->id); } return $this->zdb->execute($select)->count() > 0; @@ -420,11 +322,11 @@ private function joinActivityGroup(): void } /** - * Get activity id + * Get subscription id */ public function getId(): ?int { - return $this->id ?? null; + return $this->id; } /** @@ -432,30 +334,41 @@ public function getId(): ?int */ public function getActivityId(): ?int { - return $this->id_activity ?? null; + return $this->id_activity; } /** - * Get activity + * Get activity, loaded once */ public function getActivity(): ?Activity { - if (isset($this->id_activity)) { - $this->activity = new Activity($this->zdb, $this->id_activity); + if ($this->id_activity === null) { + return null; + } + if ($this->activity?->getId() !== $this->id_activity) { + $this->activity = new Activity($this->zdb, $this->history, $this->id_activity); } return $this->activity; } + /** + * Use an already loaded activity + * + * @param Activity $activity Activity + */ + public function useActivity(Activity $activity): self + { + $this->id_activity = $activity->getId(); + $this->activity = $activity; + return $this; + } + /** * Get amount from activity */ public function getAmountFromActivity(): ?float { - $activity = $this->getActivity(); - if ($activity !== null) { - return $this->getActivity()->getPrice(); - } - return null; + return $this->getActivity()?->getPrice(); } /** @@ -463,20 +376,35 @@ public function getAmountFromActivity(): ?float */ public function getMemberId(): ?int { - return $this->id_member ?? null; + return $this->id_member; } /** - * Get member + * Get member, loaded once */ public function getMember(): ?Adherent { - if (isset($this->id_member)) { - $this->member = new Adherent($this->zdb, $this->id_member); + if ($this->id_member === null) { + return null; + } + if ($this->member?->id !== $this->id_member) { + $this->member = new Adherent($this->zdb, $this->id_member, false); } return $this->member; } + /** + * Use an already loaded member + * + * @param Adherent $member Member + */ + public function useMember(Adherent $member): self + { + $this->id_member = $member->id; + $this->member = $member; + return $this; + } + /** * Is subscription paid? */ @@ -511,45 +439,27 @@ public function getPaymentMethodName(): string } /** - * Get creation date - * - * @param bool $formatted Return date formatted, raw if false - */ - public function getCreationDate(bool $formatted = true): string - { - return $this->getDate('creation_date', $formatted) ?? ''; - } - - /** - * Get subscription date - * - * @param bool $formatted Return date formatted, raw if false + * Get creation date, as Y-m-d */ - public function getSubscriptionDate(bool $formatted = true): string + public function getCreationDate(): string { - return $this->getDate('subscription_date', $formatted) ?? ''; + return $this->creation_date ?? ''; } /** - * Get end date - * - * @param bool $formatted Return date formatted, raw if false + * Get subscription date, as Y-m-d */ - public function getEndDate(bool $formatted = true): string + public function getSubscriptionDate(): string { - return $this->getDate('end_date', $formatted) ?? ''; + return $this->subscription_date ?? ''; } /** - * Set activity - * - * @param int $activity Activity id + * Get end date, as Y-m-d */ - public function setActivity(int $activity): self + public function getEndDate(): string { - $this->id_activity = $activity; - $this->activity = new Activity($this->zdb, $this->id_activity); - return $this; + return $this->end_date ?? ''; } /** @@ -560,18 +470,9 @@ public function setActivity(int $activity): self public function setMember(int $member): self { $this->id_member = $member; - $this->member = new Adherent($this->zdb, $this->id_member, false); return $this; } - /** - * Get table's name - */ - protected function getTableName(): string - { - return ACTIVITIES_PREFIX . self::TABLE; - } - /** * Get comment */ @@ -642,14 +543,4 @@ protected function setFields(): self return $this; } - - /** - * Get errors - * - * @return array - */ - public function getErrors(): array - { - return $this->errors; - } } diff --git a/lib/GaletteActivities/NotFoundException.php b/lib/GaletteActivities/NotFoundException.php new file mode 100644 index 0000000..e4aeb62 --- /dev/null +++ b/lib/GaletteActivities/NotFoundException.php @@ -0,0 +1,20 @@ + + */ +class NotFoundException extends \RuntimeException +{ +} diff --git a/lib/GaletteActivities/Repository/AbstractRepository.php b/lib/GaletteActivities/Repository/AbstractRepository.php new file mode 100644 index 0000000..fb39d61 --- /dev/null +++ b/lib/GaletteActivities/Repository/AbstractRepository.php @@ -0,0 +1,95 @@ + + */ +abstract class AbstractRepository extends Repository +{ + /** Primary key of listed entities */ + protected const string PK = ''; + /** Table alias used in queries */ + protected const string ALIAS = ''; + + private int $count = 0; + + /** + * Constructor + * + * @param Db $zdb Database instance + * @param Login $login Login instance + * @param History $history History instance, for listed entities + * @param Preferences $preferences Preferences instance + * @param string $entity Entity class name, relative to the plugin namespace + * @param Pagination $filters Filtering + */ + public function __construct( + Db $zdb, + Login $login, + protected History $history, + Preferences $preferences, + string $entity, + Pagination $filters + ) { + parent::__construct($zdb, $preferences, $login, $entity, 'GaletteActivities', ACTIVITIES_PREFIX); + $this->filters = $filters; + } + + /** + * Count rows matching the query + * + * @param Select $select Original select + */ + protected function proceedCount(Select $select): void + { + $counted = clone $select; + $counted->reset(Select::COLUMNS); + $counted->reset(Select::ORDER); + $counted->reset(Select::JOINS); + $counted->columns(['count' => new Expression('COUNT(DISTINCT ' . static::ALIAS . '.' . static::PK . ')')]); + foreach ($select->joins as $join) { + $counted->join($join['name'], $join['on'], [], $join['type']); + } + + $this->count = (int)$this->zdb->execute($counted)->current()['count']; + $this->filters->setCounter($this->count); + } + + /** + * Get count for current query + */ + public function getCount(): int + { + return $this->count; + } + + /** + * 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/GaletteActivities/Repository/Activities.php b/lib/GaletteActivities/Repository/Activities.php index 9e53bd7..da3d00e 100644 --- a/lib/GaletteActivities/Repository/Activities.php +++ b/lib/GaletteActivities/Repository/Activities.php @@ -11,24 +11,25 @@ namespace GaletteActivities\Repository; use Analog\Analog; -use Galette\Repository\Repository; -use GaletteActivities\Entity\Activity; +use Galette\Core\Db; +use Galette\Core\History; +use Galette\Core\Login; use Galette\Core\Preferences; +use GaletteActivities\Entity\Activity; use GaletteActivities\Filters\ActivitiesList; -use Laminas\Db\ResultSet\ResultSet; -use Laminas\Db\Sql\Expression; -use Galette\Core\Login; -use Galette\Core\Db; -use Laminas\Db\Sql\Select; /** * Activities * * @author Johan Cwiklinski */ -class Activities extends Repository +class Activities extends AbstractRepository { - private int $count; + protected const string PK = Activity::PK; + protected const string ALIAS = 'ac'; + + /** @var ActivitiesList */ + protected \Galette\Core\Pagination $filters; public const int ORDERBY_DATE = 0; public const int ORDERBY_NAME = 1; @@ -38,32 +39,29 @@ class Activities extends Repository * * @param Db $zdb Database instance * @param Login $login Login instance + * @param History $history History instance * @param Preferences $preferences Preferences instance * @param ?ActivitiesList $filters Filtering */ - public function __construct(Db $zdb, Login $login, Preferences $preferences, ?ActivitiesList $filters = null) - { - $this->zdb = $zdb; - $this->login = $login; - - parent::__construct($zdb, $preferences, $login, 'Entity\Activity', 'GaletteActivities', ACTIVITIES_PREFIX); - - if ($filters === null) { - $this->filters = new ActivitiesList(); - } else { - $this->filters = $filters; - } + public function __construct( + Db $zdb, + Login $login, + History $history, + Preferences $preferences, + ?ActivitiesList $filters = null + ) { + parent::__construct($zdb, $login, $history, $preferences, 'Entity\Activity', $filters ?? new ActivitiesList()); } /** * Get activities list * - * @return array|ResultSet + * @return array */ - public function getList(): array|ResultSet + public function getList(): array { try { - $select = $this->zdb->select(ACTIVITIES_PREFIX . Activity::TABLE, 'ac'); + $select = $this->zdb->select(ACTIVITIES_PREFIX . Activity::TABLE, self::ALIAS); $select->order($this->buildOrderClause()); $this->proceedCount($select); @@ -73,8 +71,7 @@ public function getList(): array|ResultSet $activities = []; foreach ($results as $row) { - $activity = new Activity($this->zdb, $row); - $activities[] = $activity; + $activities[] = new Activity($this->zdb, $this->history, $row); } return $activities; @@ -90,99 +87,21 @@ public function getList(): array|ResultSet /** * Builds the order clause * - * @param ?array $fields Fields list to ensure ORDER clause - * references selected fields. Optional. - * * @return array SQL ORDER clauses */ - private function buildOrderClause(?array $fields = null): array + private function buildOrderClause(): array { $order = []; switch ($this->filters->orderby) { case self::ORDERBY_DATE: - if ($this->canOrderBy('creation_date', $fields)) { - $order[] = 'creation_date ' . $this->filters->getDirection(); - } + $order[] = 'creation_date ' . $this->filters->getDirection(); break; case self::ORDERBY_NAME: - if ($this->canOrderBy('name', $fields)) { - $order[] = 'name ' . $this->filters->getDirection(); - } + $order[] = 'name ' . $this->filters->getDirection(); break; } return $order; } - - /** - * Count activities 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::ORDER); - $countSelect->reset($countSelect::HAVING); - $joins = $countSelect->joins; - $countSelect->reset($countSelect::JOINS); - foreach ($joins as $join) { - $countSelect->join( - $join['name'], - $join['on'], - [], - $join['type'] - ); - unset($join['columns']); - } - - $countSelect->columns( - [ - 'count' => new Expression('count(DISTINCT ac.' . Activity::PK . ')') - ] - ); - - $have = $select->having; - if ($have->count() > 0) { - foreach ($have->getPredicates() as $h) { - $countSelect->where($h); - } - } - - $results = $this->zdb->execute($countSelect); - - $this->count = (int)$results->current()->count; - if (isset($this->filters) && $this->count > 0) { - $this->filters->setCounter($this->count); - } - } catch (\Exception $e) { - Analog::log( - 'Cannot count activities | ' . $e->getMessage(), - Analog::WARNING - ); - throw $e; - } - } - - /** - * Get count for current query - */ - public function getCount(): int - { - return $this->count; - } - - /** - * Add default activities in database - * - * @param bool $check_first Check first if it seems initialized - */ - public function installInit(bool $check_first = true): bool - { - //to satisfy inheritance - return true; - } } diff --git a/lib/GaletteActivities/Repository/Subscriptions.php b/lib/GaletteActivities/Repository/Subscriptions.php index 04eb15b..4b15f66 100644 --- a/lib/GaletteActivities/Repository/Subscriptions.php +++ b/lib/GaletteActivities/Repository/Subscriptions.php @@ -11,25 +11,31 @@ namespace GaletteActivities\Repository; use Analog\Analog; -use Laminas\Db\Sql\Expression; use Galette\Core\Db; +use Galette\Core\History; +use Galette\Core\Login; +use Galette\Core\Preferences; use Galette\Entity\Adherent; +use Galette\Entity\Status; use GaletteActivities\Entity\Activity; use GaletteActivities\Entity\Subscription; use GaletteActivities\Filters\SubscriptionsList; +use Laminas\Db\Sql\Expression; use Laminas\Db\Sql\Select; /** - * Subscription + * Subscriptions * * @author Johan Cwiklinski */ -class Subscriptions +class Subscriptions extends AbstractRepository { - private Db $zdb; - private SubscriptionsList $filters; - private int $count; - private float $sum; + protected const string PK = Subscription::PK; + protected const string ALIAS = 's'; + + /** @var SubscriptionsList */ + protected \Galette\Core\Pagination $filters; + private float $sum = 0; public const int ORDERBY_ACTIVITY = 0; public const int ORDERBY_MEMBER = 1; @@ -45,18 +51,27 @@ class Subscriptions /** * Constructor * - * @param Db $zdb Database instance - * @param ?SubscriptionsList $filters Filtering + * @param Db $zdb Database instance + * @param Login $login Login instance + * @param History $history History instance + * @param Preferences $preferences Preferences instance + * @param ?SubscriptionsList $filters Filtering */ - public function __construct(Db $zdb, ?SubscriptionsList $filters = null) - { - $this->zdb = $zdb; - - if ($filters === null) { - $this->filters = new SubscriptionsList(); - } else { - $this->filters = $filters; - } + public function __construct( + Db $zdb, + Login $login, + History $history, + Preferences $preferences, + ?SubscriptionsList $filters = null + ) { + parent::__construct( + $zdb, + $login, + $history, + $preferences, + 'Entity\Subscription', + $filters ?? new SubscriptionsList() + ); } /** @@ -69,25 +84,27 @@ public function __construct(Db $zdb, ?SubscriptionsList $filters = null) public function getList(bool $full = false): array { try { - $select = $this->buildSelect(null); + $select = $this->buildSelect(); + $this->calculateSum($select); $this->proceedCount($select); + $select->order($this->buildOrderClause()); if ($full !== true) { $this->filters->setLimits($select); } $results = $this->zdb->execute($select); - $this->filters->query = $this->zdb->query_string; $subscriptions = []; foreach ($results as $row) { - $subscription = new Subscription($this->zdb, $row); - $subscriptions[] = $subscription; + $subscriptions[] = new Subscription($this->zdb, $this->history, $row); } + $this->loadActivities($subscriptions); + $this->loadMembers($subscriptions); return $subscriptions; } catch (\Exception $e) { Analog::log( - 'Cannot list subscription | ' . $e->getMessage(), + 'Cannot list subscriptions | ' . $e->getMessage(), Analog::WARNING ); throw $e; @@ -95,57 +112,83 @@ public function getList(bool $full = false): array } /** - * Builds the SELECT statement - * - * @param ?array $fields fields list to retrieve - * @param bool $count true if we want to count members - * (not applicable from static calls), defaults to false + * Load activities of listed subscriptions, once each * - * @return Select SELECT statement + * @param array $subscriptions Subscriptions */ - private function buildSelect(?array $fields, bool $count = false): Select + private function loadActivities(array $subscriptions): void { - try { - $fieldsList = [Subscription::PK, Activity::PK, Adherent::PK, 'is_paid', 'payment_amount', - 'payment_method', 'creation_date', 'subscription_date', 'end_date', 'comment']; - if (is_array($fields) && count($fields)) { - $fieldsList = $fields; - } - - $select = $this->zdb->select(ACTIVITIES_PREFIX . Subscription::TABLE, 's'); - $select->columns($fieldsList); + $ids = array_unique(array_map(fn(Subscription $subscription): int => (int)$subscription->getActivityId(), $subscriptions)); + if (count($ids) === 0) { + return; + } - //joined tables are used for filtering and ordering only, their columns would override subscriptions ones - $select->join( - ['a' => PREFIX_DB . Adherent::TABLE], - 's.' . Adherent::PK . '= a.' . Adherent::PK, - [] - ); - $select->join( - ['ac' => PREFIX_DB . ACTIVITIES_PREFIX . Activity::TABLE], - 's.' . Activity::PK . '= ac.' . Activity::PK, - [] - ); + $select = $this->zdb->select(ACTIVITIES_PREFIX . Activity::TABLE); + $select->where([Activity::PK => array_values($ids)]); + $activities = []; + foreach ($this->zdb->execute($select) as $row) { + $activities[(int)$row[Activity::PK]] = new Activity($this->zdb, $this->history, $row); + } - $this->buildWhereClause($select); - $select->order(self::buildOrderClause()); + foreach ($subscriptions as $subscription) { + $subscription->useActivity($activities[$subscription->getActivityId()]); + } + } - $this->calculateSum($select); + /** + * Load members of listed subscriptions, once each + * + * @param array $subscriptions Subscriptions + */ + private function loadMembers(array $subscriptions): void + { + $ids = array_unique(array_map(fn(Subscription $subscription): int => (int)$subscription->getMemberId(), $subscriptions)); + if (count($ids) === 0) { + return; + } - if ($count) { - $this->proceedCount($select); - } + //same query as a member loaded from its id + $select = $this->zdb->select(Adherent::TABLE, 'a'); + $select->join( + ['b' => PREFIX_DB . Status::TABLE], + 'a.' . Status::PK . '=b.' . Status::PK, + ['priorite_statut'] + )->where(['a.' . Adherent::PK => array_values($ids)]); + $members = []; + foreach ($this->zdb->execute($select) as $row) { + $members[(int)$row[Adherent::PK]] = new Adherent($this->zdb, $row, false); + } - return $select; - } catch (\Exception $e) { - Analog::log( - 'Cannot build SELECT clause for subscriptions | ' . $e->getMessage(), - Analog::WARNING - ); - throw $e; + foreach ($subscriptions as $subscription) { + $subscription->useMember($members[$subscription->getMemberId()]); } } + /** + * Builds the SELECT statement, filtered but neither ordered nor limited + */ + private function buildSelect(): Select + { + $select = $this->zdb->select(ACTIVITIES_PREFIX . Subscription::TABLE, self::ALIAS); + $select->columns([Subscription::PK, Activity::PK, Adherent::PK, 'is_paid', 'payment_amount', + 'payment_method', 'creation_date', 'subscription_date', 'end_date', 'comment']); + + //joined tables are used for filtering and ordering only, their columns would override subscriptions ones + $select->join( + ['a' => PREFIX_DB . Adherent::TABLE], + 's.' . Adherent::PK . '= a.' . Adherent::PK, + [] + ); + $select->join( + ['ac' => PREFIX_DB . ACTIVITIES_PREFIX . Activity::TABLE], + 's.' . Activity::PK . '= ac.' . Activity::PK, + [] + ); + + $this->buildWhereClause($select); + return $select; + } + /** * Calculate sum of all selected subscriptions * @@ -153,39 +196,9 @@ private function buildSelect(?array $fields, bool $count = false): Select */ private function calculateSum(Select $select): void { - try { - $sumSelect = clone $select; - $sumSelect->reset($sumSelect::COLUMNS); - $joins = $sumSelect->joins; - $sumSelect->reset($sumSelect::JOINS); - foreach ($joins as $join) { - $sumSelect->join( - $join['name'], - $join['on'], - [], - $join['type'] - ); - unset($join['columns']); - } - - $sumSelect->reset($sumSelect::ORDER); - $sumSelect->columns( - [ - 'sum' => new Expression('SUM(s.payment_amount)') - ] - ); - - $results = $this->zdb->execute($sumSelect); - $result = $results->current(); - - $this->sum = round((float)$result->sum, 2); - } catch (\Exception $e) { - Analog::log( - 'Cannot calculate subscriptions sum | ' . $e->getMessage(), - Analog::WARNING - ); - throw $e; - } + $sumSelect = clone $select; + $sumSelect->columns(['sum' => new Expression('SUM(s.payment_amount)')]); + $this->sum = round((float)$this->zdb->execute($sumSelect)->current()['sum'], 2); } /** @@ -270,137 +283,23 @@ private function buildWhereClause(Select $select): void } } - /** - * Is field allowed to order? it should be present in - * provided fields list (those that are SELECT'ed). - * - * @param string $field_name Field name to order by - * @param ?array $fields SELECTE'ed fields - */ - private function canOrderBy(string $field_name, ?array $fields): bool - { - if (!is_array($fields)) { - return true; - } elseif (in_array($field_name, $fields)) { - return true; - } else { - Analog::log( - 'Trying to order by ' . $field_name . ' while it is not in ' - . 'selected fields.', - Analog::WARNING - ); - return false; - } - } - /** * Builds the order clause * - * @param array $fields Fields list to ensure ORDER clause - * references selected fields. Optional. - * * @return array SQL ORDER clauses */ - private function buildOrderClause(?array $fields = null): array - { - $order = []; - - switch ($this->filters->orderby) { - case self::ORDERBY_ACTIVITY: - if ($this->canOrderBy(Activity::PK, $fields)) { - $order[] = 'ac.name ' . $this->filters->getDirection(); - } - break; - case self::ORDERBY_MEMBER: - if ($this->canOrderBy(Adherent::PK, $fields)) { - $order[] = 'a.nom_adh ' . $this->filters->getDirection(); - $order[] = 'a.prenom_adh ' . $this->filters->getDirection(); - } - break; - case self::ORDERBY_SUBSCRIPTIONDATE: - if ($this->canOrderBy('subscription_date', $fields)) { - $order[] = 's.subscription_date ' . $this->filters->getDirection(); - } - break; - - case self::ORDERBY_ENDDATE: - if ($this->canOrderBy('end_date', $fields)) { - $order[] = 's.end_date ' . $this->filters->getDirection(); - } - break; - case self::ORDERBY_PAID: - if ($this->canOrderBy('is_paid', $fields)) { - $order[] = 's.is_paid ' . $this->filters->getDirection(); - } - break; - case self::ORDERBY_AMOUNT: - if ($this->canOrderBy('payment_amount', $fields)) { - $order[] = 's.payment_amount ' . $this->filters->getDirection(); - } - break; - } - - return $order; - } - - /** - * Count activities 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::ORDER); - $countSelect->reset($countSelect::HAVING); - $joins = $countSelect->joins; - $countSelect->reset($countSelect::JOINS); - foreach ($joins as $join) { - $countSelect->join( - $join['name'], - $join['on'], - [], - $join['type'] - ); - unset($join['columns']); - } - - $countSelect->columns( - [ - 'count' => new Expression('count(DISTINCT s.' . Subscription::PK . ')') - ] - ); - - $have = $select->having; - if ($have->count() > 0) { - foreach ($have->getPredicates() as $h) { - $countSelect->where($h); - } - } - - $results = $this->zdb->execute($countSelect); - - $this->count = (int)$results->current()->count; - if ($this->count > 0) { - $this->filters->setCounter($this->count); - } - } catch (\Exception $e) { - Analog::log( - 'Cannot count subscription | ' . $e->getMessage(), - Analog::WARNING - ); - throw $e; - } - } - - /** - * Get count for current query - */ - public function getCount(): int + private function buildOrderClause(): array { - return $this->count; + $direction = $this->filters->getDirection(); + return match ($this->filters->orderby) { + self::ORDERBY_ACTIVITY => ['ac.name ' . $direction], + self::ORDERBY_MEMBER => ['a.nom_adh ' . $direction, 'a.prenom_adh ' . $direction], + self::ORDERBY_SUBSCRIPTIONDATE => ['s.subscription_date ' . $direction], + self::ORDERBY_ENDDATE => ['s.end_date ' . $direction], + self::ORDERBY_PAID => ['s.is_paid ' . $direction], + self::ORDERBY_AMOUNT => ['s.payment_amount ' . $direction], + default => [], + }; } /** diff --git a/templates/default/activities.html.twig b/templates/default/activities.html.twig index 0c5f32a..f51efa4 100644 --- a/templates/default/activities.html.twig +++ b/templates/default/activities.html.twig @@ -65,7 +65,7 @@ {{ activity.getType() }} {% if activity.getPrice() %}{{ activity.getPrice()|number_format(2) }}{% endif %} - {{ activity.getCreationDate() }} + {{ activity.getCreationDate()|date(_T("Y-m-d")) }} {% if activity.getGroup() %}{{ activity.getGroup().getFullName() }}{% endif %} {% set actions = [ diff --git a/templates/default/activity.html.twig b/templates/default/activity.html.twig index dff8e6c..10d9b8f 100644 --- a/templates/default/activity.html.twig +++ b/templates/default/activity.html.twig @@ -37,10 +37,7 @@ {% set group_list_values = group_list_values + {(group.getId()): group.getIndentName()} %} {% endfor %} - {% set group_id = null %} - {% if activity.getGroup() is not null %} - {% set group_id = activity.getGroup().getID() %} - {% endif %} + {% set group_id = activity.getGroupId() %} {% include "components/forms/select.html.twig" with { id: 'id_group', value: group_id, diff --git a/templates/default/subscription.html.twig b/templates/default/subscription.html.twig index 25143b1..412c1b5 100644 --- a/templates/default/subscription.html.twig +++ b/templates/default/subscription.html.twig @@ -16,21 +16,21 @@
{% include "components/forms/date.html.twig" with { id: 'creation_date', - value: subscription.getCreationDate(), + value: subscription.getCreationDate() ? subscription.getCreationDate()|date(_T("Y-m-d")) : '', label: _T("Creation date", "activities"), required: true } %} {% include "components/forms/date.html.twig" with { id: 'subscription_date', - value: subscription.getSubscriptionDate(), + value: subscription.getSubscriptionDate() ? subscription.getSubscriptionDate()|date(_T("Y-m-d")) : '', label: _T("Subscription date", "activities"), required: true } %} {% include "components/forms/date.html.twig" with { id: 'end_date', - value: subscription.getEndDate(), + value: subscription.getEndDate() ? subscription.getEndDate()|date(_T("Y-m-d")) : '', label: _T("End date", "activities"), required: true } %} diff --git a/templates/default/subscriptions.html.twig b/templates/default/subscriptions.html.twig index f47adf3..4b046ac 100644 --- a/templates/default/subscriptions.html.twig +++ b/templates/default/subscriptions.html.twig @@ -196,8 +196,8 @@ {{ subscription.getMember().sfullname }} - {{ subscription.getSubscriptionDate() }} - {{ subscription.getEndDate() }} + {{ subscription.getSubscriptionDate()|date(_T("Y-m-d")) }} + {{ subscription.getEndDate()|date(_T("Y-m-d")) }} {{ subscription.getAmount() }} diff --git a/tests/ActivitiesFixtures.php b/tests/ActivitiesFixtures.php index f625ce8..1bd2e26 100644 --- a/tests/ActivitiesFixtures.php +++ b/tests/ActivitiesFixtures.php @@ -124,6 +124,21 @@ protected function insertSubscription(int $activity, int $member, array $data = return (int)$this->zdb->execute($select)->current()[Subscription::PK]; } + /** + * Assert an entity does not exist + * + * @param callable $load Entity loading + */ + protected function assertNotFound(callable $load): void + { + try { + $load(); + $this->fail('Entity should not exist'); + } catch (\GaletteActivities\NotFoundException) { + $this->addToAssertionCount(1); + } + } + /** * Count subscriptions of an activity * diff --git a/tests/GaletteActivities/Controllers/Crud/tests/units/ActivitiesController.php b/tests/GaletteActivities/Controllers/Crud/tests/units/ActivitiesController.php index 2bcf25e..6aeb19d 100644 --- a/tests/GaletteActivities/Controllers/Crud/tests/units/ActivitiesController.php +++ b/tests/GaletteActivities/Controllers/Crud/tests/units/ActivitiesController.php @@ -71,7 +71,7 @@ public function testStoreError(): void $test_response->getHeaders() ); $this->expectLogEntry(\Analog\Analog::ERROR, 'Query error'); - $this->expectLogEntry(\Analog\Analog::ERROR, 'Something went wrong'); + $this->expectLogEntry(\Analog\Analog::ERROR, 'Unable to store activity #new | '); $this->expectNoLogEntry(); $this->expectFlashData(['error_detected' => ['An error occurred while storing the activity.']]); } @@ -232,7 +232,7 @@ public function testEdit(): void ); $this->expectNoLogEntry(); $this->expectFlashData(['success_detected' => ['Activity has been modified.']]); - $activity = new \GaletteActivities\Entity\Activity($this->zdb, $id); + $activity = new \GaletteActivities\Entity\Activity($this->zdb, $this->history, $id); $this->assertSame('Bouldering', $activity->getName()); $this->assertSame(8.0, $activity->getPrice()); @@ -243,14 +243,17 @@ public function testEdit(): void $this->expectLogEntry(\Analog\Analog::ERROR, 'Type is too long'); $this->expectNoLogEntry(); $this->expectFlashData(['error_detected' => ['Type is too long']]); - $this->assertSame('', (new \GaletteActivities\Entity\Activity($this->zdb, $id))->getType()); + $this->assertSame('', (new \GaletteActivities\Entity\Activity($this->zdb, $this->history, $id))->getType()); - //form is displayed again from session + //form is displayed again from session, which keeps posted values, not the entity and its database connection $this->assertNotNull($this->session->plugin_activities_activity); + $this->assertStringNotContainsString(\Galette\Core\Db::class, serialize($this->session->plugin_activities_activity)); $test_response = $this->app->handle($this->createRequest('activities_activity_edit', ['id' => (string)$id])); $this->assertSame(200, $test_response->getStatusCode()); $this->assertStringContainsString('value="Bouldering"', (string)$test_response->getBody()); $this->assertFalse(isset($this->session->plugin_activities_activity)); + //posted values are checked again + $this->expectLogEntry(\Analog\Analog::ERROR, 'Type is too long'); $this->expectNoLogEntry(); } @@ -276,7 +279,7 @@ public function testRemove(): void ); $this->expectNoLogEntry(); $this->expectFlashData(['success_detected' => ['Successfully deleted!']]); - $this->assertFalse((new \GaletteActivities\Entity\Activity($this->zdb))->load($id)); + $this->assertNotFound(fn() => (new \GaletteActivities\Entity\Activity($this->zdb, $this->history))->load($id)); $this->assertSame(0, $this->countSubscriptions($id)); } } diff --git a/tests/GaletteActivities/Controllers/Crud/tests/units/SubscriptionsController.php b/tests/GaletteActivities/Controllers/Crud/tests/units/SubscriptionsController.php index 7b215b9..cf8227f 100644 --- a/tests/GaletteActivities/Controllers/Crud/tests/units/SubscriptionsController.php +++ b/tests/GaletteActivities/Controllers/Crud/tests/units/SubscriptionsController.php @@ -179,6 +179,22 @@ public function testRemoveSubscription(): void $this->assertSame(0, $this->countSubscriptions($activity)); } + /** + * Removal of an unknown subscription is not confirmed + */ + public function testConfirmRemoveUnknownSubscription(): void + { + $this->logSuperAdmin(); + $id = $this->insertSubscription($this->insertActivity('Climbing'), $this->getMemberOne()->id) + 1000; + + $test_response = $this->app->handle( + $this->createRequest('activities_remove_subscription', ['id' => (string)$id]) + ); + $this->assertSame(200, $test_response->getStatusCode()); + $this->assertStringContainsString('No subscription #' . $id . '.', (string)$test_response->getBody()); + $this->expectNoLogEntry(); + } + /** * Member filter is optional, and can be cleared */ @@ -300,7 +316,6 @@ public function testReloadForm(): void ['Location' => [$this->routeparser->urlFor('activities_subscription_add')]], $test_response->getHeaders() ); - $this->expectLogEntry(\Analog\Analog::ERROR, 'Subscription date is mandatory'); $this->expectNoLogEntry(); $this->expectFlashData(['warning_detected' => ['Do not forget to store the subscription']]); $this->assertSame(0, $this->countSubscriptions($activity)); @@ -311,6 +326,8 @@ public function testReloadForm(): void $this->assertMatchesRegularExpression('/assertStringContainsString('placeholder="12.5"', $body); $this->assertFalse(isset($this->session->plugin_activities_subscription)); + //reloaded form is checked, not stored + $this->expectLogEntry(\Analog\Analog::ERROR, 'Subscription date is mandatory'); $this->expectNoLogEntry(); } @@ -346,7 +363,7 @@ public function testEdit(): void $this->expectNoLogEntry(); $this->expectFlashData(['success_detected' => ['Subscription has been modified.']]); - $subscription = new \GaletteActivities\Entity\Subscription($this->zdb, $id); + $subscription = new \GaletteActivities\Entity\Subscription($this->zdb, $this->history, $id); $this->assertSame('Changed comment', $subscription->getComment()); $this->assertSame(7.0, $subscription->getAmount()); $this->assertTrue($subscription->isPaid()); diff --git a/tests/GaletteActivities/Entity/tests/units/Activity.php b/tests/GaletteActivities/Entity/tests/units/Activity.php index ce69008..c723b3f 100644 --- a/tests/GaletteActivities/Entity/tests/units/Activity.php +++ b/tests/GaletteActivities/Entity/tests/units/Activity.php @@ -45,7 +45,7 @@ public function tearDown(): void */ public function testEmpty(): void { - $activity = new \GaletteActivities\Entity\Activity($this->zdb); + $activity = new \GaletteActivities\Entity\Activity($this->zdb, $this->history); $this->assertNull($activity->getId()); $this->assertSame('', $activity->getName()); @@ -59,8 +59,8 @@ public function testEmpty(): void */ public function testCrud(): void { - $activity = new \GaletteActivities\Entity\Activity($this->zdb); - $activities = new \GaletteActivities\Repository\Activities($this->zdb, $this->login, $this->preferences); + $activity = new \GaletteActivities\Entity\Activity($this->zdb, $this->history); + $activities = new \GaletteActivities\Repository\Activities($this->zdb, $this->login, $this->history, $this->preferences); //ensure the table is empty $this->assertCount(0, $activities->getList()); @@ -96,11 +96,11 @@ public function testCrud(): void 'type' => 'one' ]; $this->assertTrue($activity->check($data)); - $this->assertTrue($activity->store()); + $activity->store(); $first_id = $activity->getId(); $this->assertGreaterThan(0, $first_id); - $this->assertTrue($activity->load($first_id)); + $activity->load($first_id); $this->assertSame('Test activity', $activity->getName()); $this->assertSame('Test comment', $activity->getComment()); $this->assertSame('one', $activity->getType()); @@ -121,8 +121,8 @@ public function testCrud(): void $data['price'] = 10.5; $data['comment'] = ''; $this->assertTrue($activity->check($data)); - $this->assertTrue($activity->store()); - $this->assertTrue($activity->load($first_id)); + $activity->store(); + $activity->load($first_id); $this->assertSame('Test activity edited', $activity->getName()); $this->assertSame(10.5, $activity->getPrice()); @@ -130,18 +130,18 @@ public function testCrud(): void $group = new \Galette\Entity\Group(); $group->setName('Test group' . $this->seed); - $this->assertTrue($group->store()); + $group->store(); $data['id_group'] = $group->getId(); $this->assertTrue($activity->check($data)); - $this->assertTrue($activity->store()); - $activity = new \GaletteActivities\Entity\Activity($this->zdb, $first_id); + $activity->store(); + $activity = new \GaletteActivities\Entity\Activity($this->zdb, $this->history, $first_id); $this->assertInstanceOf(\Galette\Entity\Group::class, $activity->getGroup()); $this->assertSame($group->getId(), $activity->getGroup()->getId()); //remove activity - $this->assertTrue($activity->remove()); - $this->assertFalse($activity->load($first_id)); + $activity->remove(); + $this->assertNotFound(fn() => $activity->load($first_id)); } /** @@ -149,8 +149,9 @@ public function testCrud(): void */ public function testLoadError(): void { - $activity = new \GaletteActivities\Entity\Activity($this->zdb); - $this->assertFalse($activity->load(999)); + $activity = new \GaletteActivities\Entity\Activity($this->zdb, $this->history); + $this->expectException(\GaletteActivities\NotFoundException::class); + $activity->load(999); } /** @@ -161,7 +162,7 @@ public function testLoadError(): void */ private function expectInvalid(array $data, array $errors): void { - $activity = new \GaletteActivities\Entity\Activity($this->zdb); + $activity = new \GaletteActivities\Entity\Activity($this->zdb, $this->history); $this->assertFalse($activity->check($data)); $this->assertSame($errors, $activity->getErrors()); $this->expectLogEntry(\Analog\Analog::ERROR, 'Error(s) checking activity before store'); @@ -174,10 +175,10 @@ public function testCheckLengths(): void { $this->expectInvalid(['name' => str_repeat('a', 151)], ['Name is too long']); - $activity = new \GaletteActivities\Entity\Activity($this->zdb); + $activity = new \GaletteActivities\Entity\Activity($this->zdb, $this->history); $this->assertTrue($activity->check(['name' => str_repeat('é', 150), 'type' => 'Éàü'])); - $this->assertTrue($activity->store()); - $this->assertTrue($activity->load((int)$activity->getId())); + $activity->store(); + $activity->load((int)$activity->getId()); $this->assertSame(str_repeat('é', 150), $activity->getName()); $this->assertSame('Éàü', $activity->getType()); } @@ -187,7 +188,7 @@ public function testCheckLengths(): void */ public function testCheckPrice(): void { - $activity = new \GaletteActivities\Entity\Activity($this->zdb); + $activity = new \GaletteActivities\Entity\Activity($this->zdb, $this->history); $this->assertTrue($activity->check(['name' => 'Climbing', 'price' => '12,50'])); $this->assertSame(12.5, $activity->getPrice()); @@ -212,9 +213,9 @@ public function testRemoveCascades(): void $this->insertSubscription($climbing, $member_one->id); $this->insertSubscription($hiking, $member_one->id); - $activity = new \GaletteActivities\Entity\Activity($this->zdb, $climbing); + $activity = new \GaletteActivities\Entity\Activity($this->zdb, $this->history, $climbing); $this->assertSame($group->getId(), $activity->getGroup()?->getId()); - $this->assertTrue($activity->remove()); + $activity->remove(); $this->assertSame(0, $this->countSubscriptions($climbing)); $this->assertSame(1, $this->countSubscriptions($hiking)); diff --git a/tests/GaletteActivities/Entity/tests/units/Subscription.php b/tests/GaletteActivities/Entity/tests/units/Subscription.php index 414a909..50ead96 100644 --- a/tests/GaletteActivities/Entity/tests/units/Subscription.php +++ b/tests/GaletteActivities/Entity/tests/units/Subscription.php @@ -49,7 +49,7 @@ public function tearDown(): void */ public function testEmpty(): void { - $subscription = new \GaletteActivities\Entity\Subscription($this->zdb); + $subscription = new \GaletteActivities\Entity\Subscription($this->zdb, $this->history); $this->assertNull($subscription->getId()); $this->assertNull($subscription->getActivityId()); @@ -67,32 +67,41 @@ public function testEmpty(): void $this->assertSame('subscription-paid', $subscription->getRowClass()); } + /** + * Errors are empty before any check + */ + public function testErrorsBeforeCheck(): void + { + $subscription = new \GaletteActivities\Entity\Subscription($this->zdb, $this->history); + $this->assertSame([], $subscription->getErrors()); + } + /** * Test add and update */ public function testCrud(): void { - $subscription = new \GaletteActivities\Entity\Subscription($this->zdb); - $subscriptions = new \GaletteActivities\Repository\Activities($this->zdb, $this->login, $this->preferences); + $subscription = new \GaletteActivities\Entity\Subscription($this->zdb, $this->history); + $subscriptions = new \GaletteActivities\Repository\Activities($this->zdb, $this->login, $this->history, $this->preferences); //ensure the table is empty $this->assertCount(0, $subscriptions->getList()); //bootstrap data - $activity = new \GaletteActivities\Entity\Activity($this->zdb); + $activity = new \GaletteActivities\Entity\Activity($this->zdb, $this->history); $data = [ 'name' => 'Activity for subscriptions', 'comment' => 'Comment ' . $this->seed, 'price' => 42.0, ]; $this->assertTrue($activity->check($data)); - $this->assertTrue($activity->store()); + $activity->store(); $group = new \Galette\Entity\Group(); $group->setName('Subscribed group'); - $this->assertTrue($group->store()); + $group->store(); - $gactivity = new \GaletteActivities\Entity\Activity($this->zdb); + $gactivity = new \GaletteActivities\Entity\Activity($this->zdb, $this->history); $data = [ 'name' => 'Activity with a group', 'comment' => 'Comment for group/activity ' . $this->seed, @@ -100,7 +109,7 @@ public function testCrud(): void \Galette\Entity\Group::PK => $group->getId() ]; $this->assertTrue($gactivity->check($data)); - $this->assertTrue($gactivity->store()); + $gactivity->store(); $activity_id = $activity->getId(); $gactivity_id = $gactivity->getId(); @@ -188,7 +197,7 @@ public function testCrud(): void $this->assertSame(42.0, $subscription->getAmount()); $this->assertSame(42.0, $subscription->getAmountFromActivity()); $this->assertSame([], $subscription->getErrors()); - $this->assertTrue($subscription->store()); + $subscription->store(); $subscription_id = $subscription->getId(); //member is not part of any group @@ -199,10 +208,6 @@ public function testCrud(): void $this->assertSame(42.0, $subscription->getAmount()); $this->assertSame('2024-08-17', $subscription->getSubscriptionDate()); $this->assertSame('2025-08-17', $subscription->getEndDate()); - $this->i18n->changeLanguage('fr_FR'); - $this->assertSame('17/08/2024', $subscription->getSubscriptionDate()); - $this->assertSame('17/08/2025', $subscription->getEndDate()); - $this->i18n->changeLanguage('en_US'); $this->assertSame('subscription-notpaid', $subscription->getRowClass()); $this->assertSame($activity_id, $subscription->getActivityId()); $this->assertSame($member_one->id, $subscription->getMemberId()); @@ -210,7 +215,7 @@ public function testCrud(): void $this->assertSame($member_one->id, $subscription->getMember()->id); //reload - $subscription = new \GaletteActivities\Entity\Subscription($this->zdb, $subscription_id); + $subscription = new \GaletteActivities\Entity\Subscription($this->zdb, $this->history, $subscription_id); $data += [ 'paid' => 0, 'payment_amount' => 21.0, @@ -219,7 +224,7 @@ public function testCrud(): void $this->assertTrue($subscription->check($data)); $this->assertSame(21.0, $subscription->getAmount()); $this->assertSame(42.0, $subscription->getAmountFromActivity()); - $this->assertTrue($subscription->store()); + $subscription->store(); $this->assertFalse($subscription->isPaid()); $this->assertSame(21.0, $subscription->getAmount()); @@ -228,11 +233,11 @@ public function testCrud(): void $this->assertSame('Cash', $subscription->getPaymentMethodName()); //remove subscription - $this->assertTrue($subscription->remove()); - $this->assertFalse((new \GaletteActivities\Entity\Subscription($this->zdb))->load($subscription_id)); + $subscription->remove(); + $this->assertNotFound(fn() => (new \GaletteActivities\Entity\Subscription($this->zdb, $this->history))->load($subscription_id)); //create a subscription with a group - $subscription = new \GaletteActivities\Entity\Subscription($this->zdb); + $subscription = new \GaletteActivities\Entity\Subscription($this->zdb, $this->history); $data = [ 'activity' => $gactivity_id, 'member' => $member_one->id, @@ -241,7 +246,7 @@ public function testCrud(): void 'comment' => 'Comment ' . $this->seed, ]; $this->assertTrue($subscription->check($data)); - $this->assertTrue($subscription->store()); + $subscription->store(); //member is part activity linked group $member_one->loadGroups(); @@ -249,7 +254,7 @@ public function testCrud(): void $this->assertCount(1, $groups); //no duplicate on subscriptions - $subscription = new \GaletteActivities\Entity\Subscription($this->zdb); + $subscription = new \GaletteActivities\Entity\Subscription($this->zdb, $this->history); $data = [ 'activity' => $gactivity_id, 'member' => $member_one->id, @@ -257,14 +262,17 @@ public function testCrud(): void 'end_date' => (new \DateTime())->modify('+1 year')->format('Y-m-d'), 'comment' => 'Comment ' . $this->seed, ]; - $this->assertTrue($subscription->check($data)); - $this->assertFalse($subscription->store()); + $this->assertFalse($subscription->check($data)); $this->assertSame( [ 'Subscription already exists for this member and activity' ], $subscription->getErrors() ); + $this->expectLogEntry( + \Analog\Analog::ERROR, + 'Subscription already exists for this member and activity', + ); } /** @@ -272,8 +280,9 @@ public function testCrud(): void */ public function testLoadError(): void { - $subscription = new \GaletteActivities\Entity\Subscription($this->zdb); - $this->assertFalse($subscription->load(999)); + $subscription = new \GaletteActivities\Entity\Subscription($this->zdb, $this->history); + $this->expectException(\GaletteActivities\NotFoundException::class); + $subscription->load(999); } /** @@ -288,14 +297,14 @@ private function storeSubscription( int $member, ?\GaletteActivities\Entity\Subscription $subscription = null ): \GaletteActivities\Entity\Subscription { - $subscription ??= new \GaletteActivities\Entity\Subscription($this->zdb); + $subscription ??= new \GaletteActivities\Entity\Subscription($this->zdb, $this->history); $this->assertTrue($subscription->check([ 'activity' => $activity, 'member' => $member, 'subscription_date' => date('Y-m-d'), 'end_date' => date('Y-m-d', strtotime('+1 year')), ])); - $this->assertTrue($subscription->store()); + $subscription->store(); return $subscription; } @@ -323,15 +332,15 @@ public function testDuplicateInTransaction(): void $this->insertSubscription($activity, $member_one->id); $this->zdb->connection->beginTransaction(); - $subscription = new \GaletteActivities\Entity\Subscription($this->zdb); - $this->assertTrue($subscription->check([ + $subscription = new \GaletteActivities\Entity\Subscription($this->zdb, $this->history); + $this->assertFalse($subscription->check([ 'activity' => $activity, 'member' => $member_one->id, 'subscription_date' => date('Y-m-d'), 'end_date' => date('Y-m-d', strtotime('+1 year')), ])); - $this->assertFalse($subscription->store()); $this->assertSame(['Subscription already exists for this member and activity'], $subscription->getErrors()); + $this->expectLogEntry(\Analog\Analog::ERROR, 'Subscription already exists for this member and activity'); //transaction is still usable $this->insertActivity('Hiking'); @@ -354,7 +363,7 @@ public function testChangeActivityJoinsGroup(): void $this->assertTrue($this->isInGroup($climbing_group->getId(), $member_one->id)); $this->assertFalse($this->isInGroup($hiking_group->getId(), $member_one->id)); - $subscription = new \GaletteActivities\Entity\Subscription($this->zdb, (int)$subscription->getId()); + $subscription = new \GaletteActivities\Entity\Subscription($this->zdb, $this->history, (int)$subscription->getId()); $this->storeSubscription($hiking, $member_one->id, $subscription); $this->assertTrue($this->isInGroup($hiking_group->getId(), $member_one->id)); $this->assertTrue($this->isInGroup($climbing_group->getId(), $member_one->id)); @@ -362,7 +371,7 @@ public function testChangeActivityJoinsGroup(): void //storing again without changing activity does not join the group again $this->zdb->execute($this->zdb->delete(\Galette\Entity\Group::GROUPSUSERS_TABLE)); - $subscription = new \GaletteActivities\Entity\Subscription($this->zdb, (int)$subscription->getId()); + $subscription = new \GaletteActivities\Entity\Subscription($this->zdb, $this->history, (int)$subscription->getId()); $this->storeSubscription($hiking, $member_one->id, $subscription); $this->assertFalse($this->isInGroup($hiking_group->getId(), $member_one->id)); $this->assertSame(1, $this->countSubscriptions($hiking)); @@ -376,7 +385,7 @@ public function testChangeActivityJoinsGroup(): void */ private function expectInvalid(array $data, array $errors): void { - $subscription = new \GaletteActivities\Entity\Subscription($this->zdb); + $subscription = new \GaletteActivities\Entity\Subscription($this->zdb, $this->history); $this->assertFalse($subscription->check($data + [ 'subscription_date' => date('Y-m-d'), 'end_date' => date('Y-m-d', strtotime('+1 year')), @@ -416,7 +425,7 @@ public function testCheckDates(): void ); //same day is fine - $subscription = new \GaletteActivities\Entity\Subscription($this->zdb); + $subscription = new \GaletteActivities\Entity\Subscription($this->zdb, $this->history); $this->assertTrue($subscription->check([ 'activity' => $activity, 'member' => $member_one->id, @@ -441,7 +450,7 @@ public function testCheckAmount(): void 'save' => '1', ]; - $subscription = new \GaletteActivities\Entity\Subscription($this->zdb); + $subscription = new \GaletteActivities\Entity\Subscription($this->zdb, $this->history); $this->assertTrue($subscription->check($data + ['payment_amount' => '12,50'])); $this->assertSame(12.5, $subscription->getAmount()); @@ -451,13 +460,13 @@ public function testCheckAmount(): void //activity price, from fixtures $this->assertTrue($subscription->check($data + ['payment_amount' => ''])); $this->assertSame(10.0, $subscription->getAmount()); - $this->assertTrue($subscription->store()); + $subscription->store(); - $subscription = new \GaletteActivities\Entity\Subscription($this->zdb, (int)$subscription->getId()); + $subscription = new \GaletteActivities\Entity\Subscription($this->zdb, $this->history, (int)$subscription->getId()); $this->assertTrue($subscription->check($data + ['payment_amount' => ''])); $this->assertNull($subscription->getAmount()); - $this->assertTrue($subscription->store()); - $subscription = new \GaletteActivities\Entity\Subscription($this->zdb, (int)$subscription->getId()); + $subscription->store(); + $subscription = new \GaletteActivities\Entity\Subscription($this->zdb, $this->history, (int)$subscription->getId()); $this->assertNull($subscription->getAmount()); $this->expectInvalid($data + ['payment_amount' => 'twelve'], ['Amount must be a number.']); diff --git a/tests/GaletteActivities/Repository/tests/units/Activities.php b/tests/GaletteActivities/Repository/tests/units/Activities.php index 1b98fa9..0a54c50 100644 --- a/tests/GaletteActivities/Repository/tests/units/Activities.php +++ b/tests/GaletteActivities/Repository/tests/units/Activities.php @@ -43,7 +43,7 @@ public function tearDown(): void */ private function getListNames(ActivitiesList $filters): array { - $activities = new \GaletteActivities\Repository\Activities($this->zdb, $this->login, $this->preferences, $filters); + $activities = new \GaletteActivities\Repository\Activities($this->zdb, $this->login, $this->history, $this->preferences, $filters); $names = []; foreach ($activities->getList() as $activity) { $names[] = $activity->getName(); diff --git a/tests/GaletteActivities/Repository/tests/units/Subscriptions.php b/tests/GaletteActivities/Repository/tests/units/Subscriptions.php index 75c3a6c..0b37850 100644 --- a/tests/GaletteActivities/Repository/tests/units/Subscriptions.php +++ b/tests/GaletteActivities/Repository/tests/units/Subscriptions.php @@ -43,7 +43,7 @@ public function tearDown(): void */ private function getListIds(SubscriptionsList $filters): array { - $subscriptions = new \GaletteActivities\Repository\Subscriptions($this->zdb, $filters); + $subscriptions = new \GaletteActivities\Repository\Subscriptions($this->zdb, $this->login, $this->history, $this->preferences, $filters); $ids = []; foreach ($subscriptions->getList() as $subscription) { $ids[] = $subscription->getId(); @@ -67,11 +67,32 @@ public function testListKeepsSubscriptionValues(): void ['comment' => 'Subscription comment', 'creation_date' => '2026-01-15'] ); - $subscriptions = new \GaletteActivities\Repository\Subscriptions($this->zdb); + $subscriptions = new \GaletteActivities\Repository\Subscriptions($this->zdb, $this->login, $this->history, $this->preferences); $list = $subscriptions->getList(); $this->assertCount(1, $list); $this->assertSame('Subscription comment', $list[0]->getComment()); - $this->assertSame((new \DateTime('2026-01-15'))->format(__('Y-m-d')), $list[0]->getCreationDate()); + $this->assertSame('2026-01-15', $list[0]->getCreationDate()); + } + + /** + * Listed subscriptions share their activities and members, loaded once + */ + public function testListSharesActivitiesAndMembers(): void + { + $member_one = $this->getMemberOne(); + $climbing = $this->insertActivity('Climbing'); + $this->insertSubscription($climbing, $member_one->id, ['end_date' => '2027-01-01']); + $this->insertSubscription($climbing, $this->getMemberTwo()->id, ['end_date' => '2026-01-01']); + $this->insertSubscription($this->insertActivity('Hiking'), $member_one->id, ['end_date' => '2025-01-01']); + + $list = (new \GaletteActivities\Repository\Subscriptions($this->zdb, $this->login, $this->history, $this->preferences))->getList(); + $this->assertCount(3, $list); + $this->assertSame('Climbing', $list[0]->getActivity()?->getName()); + $this->assertSame($list[0]->getActivity(), $list[0]->getActivity()); + $this->assertSame($list[0]->getActivity(), $list[1]->getActivity()); + $this->assertSame($member_one->id, $list[0]->getMember()?->id); + $this->assertSame($member_one->sfullname, $list[0]->getMember()->sfullname); + $this->assertSame($list[0]->getMember(), $list[2]->getMember()); } /** @@ -107,7 +128,7 @@ public function testPaid(): void $filters = new SubscriptionsList(); $filters->paid_filter = \GaletteActivities\Repository\Subscriptions::FILTER_PAID; $this->assertSame([$paid_two, $paid_one], $this->getListIds($filters)); - $subscriptions = new \GaletteActivities\Repository\Subscriptions($this->zdb, $filters); + $subscriptions = new \GaletteActivities\Repository\Subscriptions($this->zdb, $this->login, $this->history, $this->preferences, $filters); $subscriptions->getList(); $this->assertSame(15.5, $subscriptions->getSum()); @@ -149,7 +170,7 @@ public function testFilters(): void $filters->activity_filter = $climbing; $this->assertSame([$second, $first], $this->getListIds($filters)); - $subscriptions = new \GaletteActivities\Repository\Subscriptions($this->zdb, $filters); + $subscriptions = new \GaletteActivities\Repository\Subscriptions($this->zdb, $this->login, $this->history, $this->preferences, $filters); $subscriptions->getList(); $this->assertSame(2, $subscriptions->getCount()); From 48129aaece35eca0e72790297bb9323c8da9ef36 Mon Sep 17 00:00:00 2001 From: Johan Cwiklinski Date: Sun, 27 Sep 2026 18:13:14 +0200 Subject: [PATCH 3/3] Refuse unknown list filters --- .../Filters/ActivitiesList.php | 3 -- .../Filters/SubscriptionsList.php | 22 +++++++----- .../Filters/tests/units/SubscriptionsList.php | 34 +++++++++++++++++++ 3 files changed, 47 insertions(+), 12 deletions(-) create mode 100644 tests/GaletteActivities/Filters/tests/units/SubscriptionsList.php diff --git a/lib/GaletteActivities/Filters/ActivitiesList.php b/lib/GaletteActivities/Filters/ActivitiesList.php index 9971a9d..46d85a3 100644 --- a/lib/GaletteActivities/Filters/ActivitiesList.php +++ b/lib/GaletteActivities/Filters/ActivitiesList.php @@ -18,10 +18,7 @@ * Activities lists filters and paginator * * @author Johan Cwiklinski - * - * @property string $query */ - class ActivitiesList extends Pagination { /** diff --git a/lib/GaletteActivities/Filters/SubscriptionsList.php b/lib/GaletteActivities/Filters/SubscriptionsList.php index 8bd38d6..24df554 100644 --- a/lib/GaletteActivities/Filters/SubscriptionsList.php +++ b/lib/GaletteActivities/Filters/SubscriptionsList.php @@ -31,7 +31,6 @@ * @property int $payment_type_filter * @property int $date_field * @property array $selected - * @property string $query */ class SubscriptionsList extends Pagination { @@ -41,18 +40,17 @@ class SubscriptionsList extends Pagination public const int DATE_SUBSCRIPTION = 1; public const int DATE_CREATION = 2; //filters - private string|int|null $activity_filter; - private string|int|null $member_filter; + private ?int $activity_filter; + private ?int $member_filter; - private int|string $paid_filter; + private int $paid_filter; private int $payment_type_filter; private ?int $date_field = null; - private ?string $start_date_filter; - private ?string $end_date_filter; + private ?string $start_date_filter; //@phpstan-ignore property.unusedType (assigned by DatesHelper::setFilterDate()) + private ?string $end_date_filter; //@phpstan-ignore property.unusedType (assigned by DatesHelper::setFilterDate()) /** @var array */ private array $selected; - private string $query; /** @var array */ protected array $list_fields = [ @@ -188,13 +186,19 @@ public function __set(string $name, mixed $value): void //empty means no filter $this->$name = ($value === null || $value === '') ? null : (int)$value; break; + case 'paid_filter': case 'payment_type_filter': case 'date_field': $this->$name = (int)$value; break; default: - $this->$name = $value; - break; + throw new \RuntimeException( + sprintf( + 'Unable to set property "%s::%s"!', + __CLASS__, + $name + ) + ); } } } diff --git a/tests/GaletteActivities/Filters/tests/units/SubscriptionsList.php b/tests/GaletteActivities/Filters/tests/units/SubscriptionsList.php new file mode 100644 index 0000000..bb99ab2 --- /dev/null +++ b/tests/GaletteActivities/Filters/tests/units/SubscriptionsList.php @@ -0,0 +1,34 @@ + + */ +class SubscriptionsList extends GaletteTestCase +{ + protected int $seed = 20260927143012; + + /** + * Unknown properties are refused + */ + public function testUnknownPropertyIsRefused(): void + { + $filters = new \GaletteActivities\Filters\SubscriptionsList(); + $this->expectException(\RuntimeException::class); + $this->expectExceptionMessage('Unable to set property "GaletteActivities\Filters\SubscriptionsList::query"!'); + $filters->query = 'SELECT 1'; //@phpstan-ignore property.notFound (removed property) + } +}