Settings Modernization: Applied suggested changed from #972

resources/views/macros/base.twig

@@ -47,10 +47,6 @@
     </span>
 {%- endmacro %}
 
-{% macro entry_required(text) %}
-    <h4 class="help-block">{{ _self.icon('exclamation-triangle', 'info') }}{{ text }}</h4>
-{%- endmacro %}
-
 {% macro type_bg_class() -%}
     {% if theme.type == 'light' %}bg-white{% else %}bg-dark{% endif %}
 {%- endmacro %}

resources/views/macros/form.twig

@@ -1,10 +1,14 @@
+{% macro entry_required() %}
+    <span class="bi bi-exclamation-triangle text-info"></span>
+{%- endmacro %}
+
 {% macro input(name, label, type, opt) %}
     <div class="mb-3">
         {% if label -%}
             <label for="{{ name }}" class="form-label {% if opt.hide_label|default(false) %}sr-only{% endif %}">
                 {{ label }}
                 {% if opt.entry_required_icon|default(false) %}
-                    <span class="bi bi-exclamation-triangle text-info"></span>
+                    {{ _self.entry_required() }}
                 {% endif %}
             </label>
         {%- endif %}
@@ -48,7 +52,12 @@
 {% macro select(name, data, label, selected, opt) %}
     <div class="mb-3">
         {% if label -%}
-            <label class="form-label" for="{{ name }}">{{ label }}</label>
+            <label class="form-label" for="{{ name }}">
+                {{ label }}
+                {% if opt.entry_required_icon|default(false) %}
+                    {{ _self.entry_required() }}
+                {% endif %}
+            </label>
         {% endif %}
         <select id="{{ name }}" name="{{ name }}"
                 class="form-control {%- if opt.class is defined %} {{ opt.class }}{% endif %}"

resources/views/pages/settings/profile.twig

@@ -2,16 +2,21 @@
 {% import 'macros/form.twig' as f %}
 {% import 'macros/base.twig' as m %}
 
-{% block title %}{{ __('settings.language') }}{% endblock %}
+{% block title %}{{ __('settings.profile') }}{% endblock %}
 
 {% block row_content %}
     <form action="" enctype="multipart/form-data" method="post">
         {{ csrf() }}
 
         <div class="row g-4">
+            <div class="col-md-12">
+                <strong class="help-block">
+                    {{ f.entry_required() }} = {{ __('settings.profile.entry_required') }}
+                </strong>
+            </div>
+
             <div class="col-md-12">
                 {{ m.info(__('settings.profile.user_details.info')) }}
-                {{ m.entry_required(' = ' ~ __('settings.profile.entry_required')) }}
                 {{ f.input(
                     'nick',
                     __('settings.profile.nick'),
@@ -133,17 +138,23 @@
                 {% endif %}
             </div>
 
-            <div class="col-md-12">
+            {% if config('enable_tshirt_size') %}
-                {{ f.select(
+                <div class="col-md-12">
-                    'shirt_size',
+                    {{ f.select(
-                    config('tshirt_sizes'),
+                        'shirt_size',
-                    __('settings.profile.shirt_size'),
+                        config('tshirt_sizes'),
-                    user.personalData.shirt_size
+                        __('settings.profile.shirt_size'),
-                ) }}
+                        user.personalData.shirt_size,
-            </div>
+                        {'required': true, 'entry_required_icon': true}
+                    ) }}
+                </div>
+            {% endif %}
 
             <div class="col-md-12">
                 {{ m.info(__('settings.profile.angeltypes.info', [url('/angeltypes')]), true) }}
+            </div>
+
+            <div class="col-md-12">
                 {{ f.submit() }}
             </div>
         </div>

src/Controllers/ChecksArrivalsAndDepartures.php

@@ -7,6 +7,11 @@ use DateTime;
 
 trait ChecksArrivalsAndDepartures
 {
+    /**
+     * @param string|null $arrival
+     * @param string|null $departure
+     * @return bool
+     */
     protected function isArrivalDateValid(?string $arrival, ?string $departure): bool
     {
         $arrival_carbon = $this->toCarbon($arrival);
@@ -23,6 +28,11 @@ trait ChecksArrivalsAndDepartures
         return !$this->isBeforeBuildup($arrival_carbon) && !$this->isAfterTeardown($arrival_carbon);
     }
 
+    /**
+     * @param string|null $arrival
+     * @param string|null $departure
+     * @return bool
+     */
     protected function isDepartureDateValid(?string $arrival, ?string $departure): bool
     {
         $arrival_carbon = $this->toCarbon($arrival);
@@ -45,12 +55,12 @@ trait ChecksArrivalsAndDepartures
     private function isBeforeBuildup(Carbon $date): bool
     {
         $buildup = config('buildup_start');
-        return !empty($buildup) && $date->lessThan($buildup->setTime(0, 0));
+        return !empty($buildup) && $date->lessThan($buildup->startOfDay());
     }
 
     private function isAfterTeardown(Carbon $date): bool
     {
         $teardown = config('teardown_end');
-        return !empty($teardown) && $date->greaterThanOrEqualTo($teardown->addDay()->setTime(0, 0));
+        return !empty($teardown) && $date->greaterThanOrEqualTo($teardown->endOfDay());
     }
 }

src/Controllers/SettingsController.php

@@ -36,7 +36,10 @@ class SettingsController extends BaseController
     ];
 
     /**
-     * @param Config   $config
+     * @param Authenticator $auth
+     * @param Config $config
+     * @param LoggerInterface $log
+     * @param Redirector $redirector
      * @param Response $response
      */
     public function __construct(
@@ -53,6 +56,9 @@ class SettingsController extends BaseController
         $this->response = $response;
     }
 
+    /**
+     * @return Response
+     */
     public function profile(): Response
     {
         $user = $this->auth->user();
@@ -73,26 +79,7 @@ class SettingsController extends BaseController
     public function saveProfile(Request $request): Response
     {
         $user = $this->auth->user();
-
+        $data = $this->validate($request, $this->getSaveProfileRules());
-        config('buildup_start');
-        config('teardown_end');
-
-        $data = $this->validate($request, [
-            'pronoun'                => 'optional|max:15',
-            'first_name'             => 'optional|max:64',
-            'last_name'              => 'optional|max:64',
-            'planned_arrival_date'   => 'required|date:Y-m-d',
-            'planned_departure_date' => 'optional|date:Y-m-d',
-            'dect'                   => 'optional|length:0:40', // dect/mobile can be purely numbers. "max" would have
-            'mobile'                 => 'optional|length:0:40', // checked their values, not their character length.
-            'mobile_show'            => 'optional|checked',
-            'email'                  => 'required|email|max:254',
-            'email_shiftinfo'        => 'optional|checked',
-            'email_news'             => 'optional|checked',
-            'email_human'            => 'optional|checked',
-            'email_goody'            => 'optional|checked',
-            'shirt_size'             => 'required',
-        ]);
 
         if (config('enable_pronoun')) {
             $user->personalData->pronoun = $data['pronoun'];
@@ -135,9 +122,8 @@ class SettingsController extends BaseController
             $user->settings->email_goody = $data['email_goody'] ?: false;
         }
 
-        if (isset(config('tshirt_sizes')[$data['shirt_size']])) {
+        if (config('enable_tshirt_size') && isset(config('tshirt_sizes')[$data['shirt_size']])) {
             $user->personalData->shirt_size = $data['shirt_size'];
-            $user->personalData->save();
         }
 
         $user->personalData->save();
@@ -330,4 +316,32 @@ class SettingsController extends BaseController
 
         return true;
     }
+
+    /**
+     * @return string[]
+     */
+    private function getSaveProfileRules(): array
+    {
+        $rules = [
+            'pronoun' => 'optional|max:15',
+            'first_name' => 'optional|max:64',
+            'last_name' => 'optional|max:64',
+            'dect' => 'optional|length:0:40', // dect/mobile can be purely numbers. "max" would have
+            'mobile' => 'optional|length:0:40', // checked their values, not their character length.
+            'mobile_show' => 'optional|checked',
+            'email' => 'required|email|max:254',
+            'email_shiftinfo' => 'optional|checked',
+            'email_news' => 'optional|checked',
+            'email_human' => 'optional|checked',
+            'email_goody' => 'optional|checked',
+        ];
+        if (config('enable_planned_arrival')) {
+            $rules['planned_arrival_date'] = 'required|date:Y-m-d';
+            $rules['planned_departure_date'] = 'optional|date:Y-m-d';
+        }
+        if (config('enable_tshirt_size')) {
+            $rules['shirt_size'] = 'required';
+        }
+        return $rules;
+    }
 }

tests/Unit/Controllers/ControllerTest.php

@@ -37,12 +37,12 @@ abstract class ControllerTest extends TestCase
 
     /**
      * @param string      $value
-     * @param string|null $message
+     * @param string|null $type
      */
-    protected function assertHasNotification(string $value, string $message = null)
+    protected function assertHasNotification(string $value, string $type = 'messages')
     {
-        $messages = $this->session->get('messages', []);
+        $messages = $this->session->get($type, []);
-        $this->assertTrue(in_array($value, $messages), $message ?: 'Session does not contain message "' . $value . '"');
+        $this->assertTrue(in_array($value, $messages));
     }
 
     /**

tests/Unit/Controllers/SettingsControllerTest.php

@@ -2,52 +2,31 @@
 
 namespace Engelsystem\Test\Unit\Controllers;
 
+use Carbon\Carbon;
 use Engelsystem\Config\Config;
 use Engelsystem\Controllers\SettingsController;
 use Engelsystem\Http\Exceptions\HttpNotFound;
 use Engelsystem\Http\Response;
 use Engelsystem\Models\User\Settings;
-use Engelsystem\Test\Unit\TestCase;
 use PHPUnit\Framework\MockObject\MockObject;
 use Symfony\Component\HttpFoundation\Session\Session;
-use Symfony\Component\HttpFoundation\Session\Storage\MockArraySessionStorage;
-use Psr\Log\LoggerInterface;
 use Engelsystem\Helpers\Authenticator;
 use Engelsystem\Test\Unit\HasDatabase;
-use Engelsystem\Http\Request;
-use Psr\Http\Message\ServerRequestInterface;
-use Psr\Log\Test\TestLogger;
-use Engelsystem\Http\UrlGeneratorInterface;
 use Engelsystem\Http\UrlGenerator;
 use Engelsystem\Models\User\User;
 use Engelsystem\Http\Validation\Validator;
 use Engelsystem\Http\Exceptions\ValidationException;
 
-class SettingsControllerTest extends TestCase
+class SettingsControllerTest extends ControllerTest
 {
     use HasDatabase;
 
     /** @var Authenticator|MockObject */
     protected $auth;
 
-    /** @var Config */
-    protected $config;
-
-    /** @var TestLogger */
-    protected $log;
-
-    /** @var Response|MockObject */
-    protected $response;
-
-    /** @var Request */
-    protected $request;
-
     /** @var User */
     protected $user;
 
-    /** @var Session */
-    protected $session;
-
     /** @var SettingsController */
     protected $controller;
 
@@ -85,6 +64,7 @@ class SettingsControllerTest extends TestCase
             'enable_dect'            => true,
             'enable_mobile_show'     => true,
             'enable_goody'           => true,
+            'enable_tshirt_size'     => true,
         ]);
 
         $this->setExpects($this->auth, 'user', null, $this->user, $this->atLeastOnce());
@@ -117,6 +97,7 @@ class SettingsControllerTest extends TestCase
 
     /**
      * @covers \Engelsystem\Controllers\SettingsController::saveProfile
+     * @covers \Engelsystem\Controllers\SettingsController::getSaveProfileRules
      */
     public function testSaveProfile()
     {
@@ -145,6 +126,28 @@ class SettingsControllerTest extends TestCase
         $this->assertEquals($body['shirt_size'], $this->user->personalData->shirt_size);
     }
 
+    /**
+     * @covers \Engelsystem\Controllers\SettingsController::saveProfile
+     */
+    public function testSaveProfileThrowsErrorOnInvalidArrival()
+    {
+        $this->setUpProfileTest();
+        config(['buildup_start' => new Carbon('2022-01-02')]); // arrival before buildup
+        $this->controller->saveProfile($this->request);
+        $this->assertHasNotification('settings.profile.planned_arrival_date.invalid', 'errors');
+    }
+
+    /**
+     * @covers \Engelsystem\Controllers\SettingsController::saveProfile
+     */
+    public function testSaveProfileThrowsErrorOnInvalidDeparture()
+    {
+        $this->setUpProfileTest();
+        config(['teardown_end' => new Carbon('2022-01-01')]); // departure after teardown
+        $this->controller->saveProfile($this->request);
+        $this->assertHasNotification('settings.profile.planned_departure_date.invalid', 'errors');
+    }
+
     /**
      * @covers \Engelsystem\Controllers\SettingsController::saveProfile
      */
@@ -213,6 +216,17 @@ class SettingsControllerTest extends TestCase
         $this->assertFalse($this->user->settings->email_goody);
     }
 
+    /**
+     * @covers \Engelsystem\Controllers\SettingsController::saveProfile
+     */
+    public function testSaveProfileIgnoresTShirtSizeIfDisabled()
+    {
+        $this->setUpProfileTest();
+        config(['enable_tshirt_size' => false]);
+        $this->controller->saveProfile($this->request);
+        $this->assertEquals('', $this->user->personalData->shirt_size);
+    }
+
     /**
      * @covers \Engelsystem\Controllers\SettingsController::password
      */
@@ -619,7 +633,6 @@ class SettingsControllerTest extends TestCase
     public function setUp(): void
     {
         parent::setUp();
-        $this->initDatabase();
 
         $themes = [
             0 => ['name' => 'Engelsystem light'],
@@ -639,23 +652,8 @@ class SettingsControllerTest extends TestCase
         $this->app->instance('config', $this->config);
         $this->app->instance(Config::class, $this->config);
 
-        $this->request = Request::create('http://localhost');
-        $this->app->instance('request', $this->request);
-        $this->app->instance(Request::class, $this->request);
-        $this->app->instance(ServerRequestInterface::class, $this->request);
-
-        $this->response = $this->createMock(Response::class);
-        $this->app->instance(Response::class, $this->response);
-
-        $this->app->bind(UrlGeneratorInterface::class, UrlGenerator::class);
         $this->app->bind('http.urlGenerator', UrlGenerator::class);
 
-        $this->log = new TestLogger();
-        $this->app->instance(LoggerInterface::class, $this->log);
-
-        $this->session = new Session(new MockArraySessionStorage());
-        $this->app->instance('session', $this->session);
-
         $this->auth = $this->createMock(Authenticator::class);
         $this->app->instance(Authenticator::class, $this->auth);