fix: various validation related issues (resolves accessibility-exchan…

…ge#1776, accessibility-exchange#1775, accessibility-exchange#1774, accessibility-exchange#1773, accessibility-exchange#1772) (accessibility-exchange#1820) * fix: update request validations and add tests * fix: engagement edit language field error message Also added tests and fixed errors with the validation rules. * fix: eng survey details missing inline errors * fix: failing engagement test * fix: engagement selection criteria validation * fix: meeting validation error messages * test: refactor validation tests * fix: update the validation messages * fix: typo for Indigenous group * style: remove duplicate array value * chore: update localization files
marvinroman · Jul 24, 2023 · dc7f444 · dc7f444
1 parent 61de743
commit dc7f444
Show file tree

Hide file tree

Showing 45 changed files with 2,461 additions and 176 deletions.
diff --git a/app/Enums/IdentityType.php b/app/Enums/IdentityType.php
@@ -0,0 +1,27 @@
+<?php
+
+namespace App\Enums;
+
+enum IdentityType: string
+{
+    case AgeBracket = 'age-bracket';
+    case GenderAndSexualIdentity = 'gender-and-sexual-identity';
+    case IndigenousIdentity = 'indigenous-identity';
+    case EthnoracialIdentity = 'ethnoracial-identity';
+    case RefugeeOrImmigrant = 'refugee-or-immigrant';
+    case FirstLanguage = 'first-language';
+    case AreaType = 'area-type';
+
+    public static function labels(): array
+    {
+        return [
+            'age-bracket' => __('Age'),
+            'gender-and-sexual-identity' => __('Gender and sexual identity'),
+            'indigenous-identity' => __('Indigenous'),
+            'ethnoracial-identity' => __('Race and ethnicity'),
+            'refugee-or-immigrant' => __('Refugees and/or immigrants'),
+            'first-language' => __('First language'),
+            'area-type' => __('Living in urban, rural, or remote areas'),
+        ];
+    }
+}
diff --git a/app/Enums/LocationType.php b/app/Enums/LocationType.php
@@ -0,0 +1,17 @@
+<?php
+
+namespace App\Enums;
+
+enum LocationType: string
+{
+    case Regions = 'regions';
+    case Localities = 'localities';
+
+    public static function labels(): array
+    {
+        return [
+            'regions' => __('Specific provinces or territories'),
+            'localities' => __('Specific cities or towns'),
+        ];
+    }
+}
diff --git a/app/Http/Controllers/EngagementController.php b/app/Http/Controllers/EngagementController.php
@@ -7,6 +7,8 @@
 use App\Enums\EngagementFormat;
 use App\Enums\EngagementRecruitment;
 use App\Enums\IdentityCluster;
+use App\Enums\IdentityType;
+use App\Enums\LocationType;
 use App\Enums\MeetingType;
 use App\Enums\ProvinceOrTerritory;
 use App\Enums\TimeZone;
@@ -164,23 +166,12 @@ public function criteriaSelection(Engagement $engagement): View
             'indigenousIdentities' => Options::forModels(Identity::query()->whereJsonContains('clusters', IdentityCluster::Indigenous))->toArray(),
             'ethnoracialIdentities' => Options::forModels(Identity::query()->whereJsonContains('clusters', IdentityCluster::Ethnoracial))->toArray(),
             'languages' => Options::forArray(get_available_languages(true))->nullable(__('Choose a language…'))->toArray(),
-            'locationTypeOptions' => Options::forArray([
-                'regions' => __('Specific provinces or territories'),
-                'localities' => __('Specific cities or towns'),
-            ])->toArray(),
+            'locationTypeOptions' => Options::forEnum(LocationType::class)->toArray(),
             'intersectionalOptions' => Options::forArray([
                 '1' => __('No, give me a group with intersectional experiences and/or identities'),
                 '0' => __('Yes, I’m looking for a group with a specific experience and/or identity (for example: Indigenous, immigrant, 2SLGBTQIA+)'),
             ])->toArray(),
-            'otherIdentityOptions' => Options::forArray([
-                'age-bracket' => __('Age'),
-                'gender-and-sexual-identity' => __('Gender and sexual identity'),
-                'indigenous-identity' => __('Indigenous'),
-                'ethnoracial-identity' => __('Race and ethnicity'),
-                'refugee-or-immigrant' => __('Refugees and/or immigrants'),
-                'first-language' => __('First language'),
-                'area-type' => __('Living in urban, rural, or remote areas'),
-            ])->nullable(__('Select a criteria…'))->toArray(),
+            'otherIdentityOptions' => Options::forEnum(IdentityType::class)->nullable(__('Select a criteria…'))->toArray(),
         ]);
     }
 
@@ -192,11 +183,11 @@ public function updateCriteria(UpdateEngagementSelectionCriteriaRequest $request
 
         $matchingStrategyData = $request->safe()->except(['ideal_participants', 'minimum_participants']);
 
-        if ($matchingStrategyData['location_type'] === 'regions') {
+        if ($matchingStrategyData['location_type'] === LocationType::Regions->value) {
             $matchingStrategyData['locations'] = [];
         }
 
-        if ($matchingStrategyData['location_type'] === 'localities') {
+        if ($matchingStrategyData['location_type'] === LocationType::Localities->value) {
             $matchingStrategyData['regions'] = [];
         }
 
@@ -215,7 +206,7 @@ public function updateCriteria(UpdateEngagementSelectionCriteriaRequest $request
 
         if ($matchingStrategyData['intersectional'] == 0) {
             $matchingStrategy->extra_attributes->other_identity_type = $matchingStrategyData['other_identity_type'];
-            if ($matchingStrategyData['other_identity_type'] === 'age-bracket') {
+            if ($matchingStrategyData['other_identity_type'] === IdentityType::AgeBracket->value) {
                 $matchingStrategy->languages()->detach();
                 $matchingStrategy->syncMutuallyExclusiveIdentities(
                     IdentityCluster::Age,
@@ -229,7 +220,7 @@ public function updateCriteria(UpdateEngagementSelectionCriteriaRequest $request
                 );
             }
 
-            if ($matchingStrategyData['other_identity_type'] === 'gender-and-sexual-identity') {
+            if ($matchingStrategyData['other_identity_type'] === IdentityType::GenderAndSexualIdentity->value) {
                 if (isset($matchingStrategyData['nb_gnc_fluid_identity']) && $matchingStrategyData['nb_gnc_fluid_identity'] == 1) {
                     foreach (Identity::whereJsonContains('clusters', IdentityCluster::GenderDiverse)->get() as $genderDiverseIdentity) {
                         $matchingStrategyData['gender_and_sexual_identities'][] = $genderDiverseIdentity->id;
@@ -250,7 +241,7 @@ public function updateCriteria(UpdateEngagementSelectionCriteriaRequest $request
                 );
             }
 
-            if ($matchingStrategyData['other_identity_type'] === 'indigenous-identity') {
+            if ($matchingStrategyData['other_identity_type'] === IdentityType::IndigenousIdentity->value) {
                 $matchingStrategy->languages()->detach();
                 $matchingStrategy->syncMutuallyExclusiveIdentities(
                     IdentityCluster::Indigenous,
@@ -264,7 +255,7 @@ public function updateCriteria(UpdateEngagementSelectionCriteriaRequest $request
                 );
             }
 
-            if ($matchingStrategyData['other_identity_type'] === 'ethnoracial-identity') {
+            if ($matchingStrategyData['other_identity_type'] === IdentityType::EthnoracialIdentity->value) {
                 $matchingStrategy->languages()->detach();
                 $matchingStrategy->syncMutuallyExclusiveIdentities(
                     IdentityCluster::Ethnoracial,
@@ -279,7 +270,7 @@ public function updateCriteria(UpdateEngagementSelectionCriteriaRequest $request
                 );
             }
 
-            if ($matchingStrategyData['other_identity_type'] === 'refugee-or-immigrant') {
+            if ($matchingStrategyData['other_identity_type'] === IdentityType::RefugeeOrImmigrant->value) {
                 $matchingStrategy->languages()->detach();
                 $matchingStrategy->detachClusters([
                     IdentityCluster::Age,
@@ -294,7 +285,7 @@ public function updateCriteria(UpdateEngagementSelectionCriteriaRequest $request
                 );
             }
 
-            if ($matchingStrategyData['other_identity_type'] === 'first-language') {
+            if ($matchingStrategyData['other_identity_type'] === IdentityType::FirstLanguage->value) {
                 $languages = [];
 
                 foreach ($matchingStrategyData['first_languages'] as $code) {
@@ -322,7 +313,7 @@ public function updateCriteria(UpdateEngagementSelectionCriteriaRequest $request
                 ]);
             }
 
-            if ($matchingStrategyData['other_identity_type'] === 'area-type') {
+            if ($matchingStrategyData['other_identity_type'] === IdentityType::AreaType->value) {
                 $matchingStrategy->languages()->detach();
                 $matchingStrategy->syncMutuallyExclusiveIdentities(
                     IdentityCluster::Area,

diff --git a/app/Http/Requests/AddNotificationableRequest.php b/app/Http/Requests/AddNotificationableRequest.php
@@ -8,6 +8,10 @@ class AddNotificationableRequest extends FormRequest
 {
     public function authorize(): bool
     {
+        if (! is_callable($this->input('notificationable_type').'::find')) {
+            return false;
+        }
+
         $notificationable = $this->input('notificationable_type')::find($this->input('notificationable_id'));
 
         return $notificationable && $this->user()->can('receiveNotifications');

diff --git a/app/Http/Requests/AddTranslationRequest.php b/app/Http/Requests/AddTranslationRequest.php
@@ -12,6 +12,10 @@ class AddTranslationRequest extends FormRequest
      */
     public function authorize(): bool
     {
+        if (! is_callable($this->input('translatable_type').'::where')) {
+            return false;
+        }
+
         $translatable = $this->input('translatable_type')::where('id', $this->input('translatable_id'))->first();
 
         return $translatable && $this->user()->can('update', $translatable);
@@ -39,7 +43,9 @@ public function rules(): array
     public function messages(): array
     {
         return [
-            'new_language.not_in' => __(':model is already translatable into :language.', ['model' => $this->get('translatable_type')::find($this->get('translatable_id'))->name, 'language' => get_language_exonym($this->get('new_language'))]),
+            'new_language.required' => __('Please select a language.', ['model' => $this->get('translatable_type')::find($this->get('translatable_id'))->name]),
+            'new_language.string' => __('Please select a language.', ['model' => $this->get('translatable_type')::find($this->get('translatable_id'))->name]),
+            'new_language.not_in' => __(':model is already translatable into :language.', ['model' => $this->get('translatable_type')::find($this->get('translatable_id'))->name, 'language' => get_language_exonym($this->get('new_language') ?? '')]),
         ];
     }
 }
diff --git a/app/Http/Requests/DestroyTranslationRequest.php b/app/Http/Requests/DestroyTranslationRequest.php
@@ -12,6 +12,10 @@ class DestroyTranslationRequest extends FormRequest
      */
     public function authorize(): bool
     {
+        if (! is_callable($this->input('translatable_type').'::where')) {
+            return false;
+        }
+
         $translatable = $this->input('translatable_type')::where('id', $this->input('translatable_id'))->first();
 
         return $translatable && $this->user()->can('update', $translatable);
@@ -39,7 +43,9 @@ public function rules(): array
     public function messages(): array
     {
         return [
-            'language.in' => __(':model was not translatable into :language.', ['model' => $this->get('translatable_type')::find($this->get('translatable_id'))->name, 'language' => get_language_exonym($this->get('language'))]),
+            'language.required' => __('Please select a language to remove.'),
+            'language.string' => __('Please select a language to remove.'),
+            'language.in' => __(':model was not translatable into :language.', ['model' => $this->get('translatable_type')::find($this->get('translatable_id'))->name, 'language' => get_language_exonym($this->get('language') ?? '')]),
         ];
     }
 }
diff --git a/app/Http/Requests/MeetingRequest.php b/app/Http/Requests/MeetingRequest.php
@@ -7,9 +7,12 @@
 use Illuminate\Foundation\Http\FormRequest;
 use Illuminate\Validation\Rule;
 use Illuminate\Validation\Rules\Enum;
+use Worksome\RequestFactories\Concerns\HasFactory;
 
 class MeetingRequest extends FormRequest
 {
+    use HasFactory;
+
     public function authorize(): bool
     {
         return true;
@@ -22,12 +25,12 @@ public function rules(): array
         $phone = MeetingType::Phone->value;
 
         return [
-            'title.*' => 'nullable|string',
             'title.en' => 'required_without:title.fr',
             'title.fr' => 'required_without:title.en',
+            'title.*' => 'nullable|string',
             'date' => 'required|date',
-            'start_time' => 'required|date_format:G:i',
-            'end_time' => 'required|date_format:G:i',
+            'start_time' => 'required|date_format:G:i|before:end_time',
+            'end_time' => 'required|date_format:G:i|after:start_time',
             'timezone' => 'required|timezone',
             'meeting_types' => 'required|array',
             'meeting_types.*' => [
@@ -36,67 +39,69 @@ public function rules(): array
             ],
             'street_address' => [
                 'nullable',
-                Rule::excludeIf(! in_array($inPerson, $this->input('meeting_types', []))),
-                Rule::requiredIf(in_array($inPerson, $this->input('meeting_types', []))),
+                Rule::excludeIf(! in_array($inPerson, is_array($this->input('meeting_types')) ? $this->input('meeting_types') : [])),
+                Rule::requiredIf(in_array($inPerson, is_array($this->input('meeting_types')) ? $this->input('meeting_types') : [])),
                 'string',
             ],
             'unit_suite_floor' => [
                 'nullable',
-                Rule::excludeIf(! in_array($inPerson, $this->input('meeting_types', []))),
+                Rule::excludeIf(! in_array($inPerson, is_array($this->input('meeting_types')) ? $this->input('meeting_types') : [])),
                 'string',
             ],
             'locality' => [
                 'nullable',
-                Rule::excludeIf(! in_array($inPerson, $this->input('meeting_types', []))),
-                Rule::requiredIf(in_array($inPerson, $this->input('meeting_types', []))),
+                Rule::excludeIf(! in_array($inPerson, is_array($this->input('meeting_types')) ? $this->input('meeting_types') : [])),
+                Rule::requiredIf(in_array($inPerson, is_array($this->input('meeting_types')) ? $this->input('meeting_types') : [])),
                 'string',
             ],
             'region' => [
                 'nullable',
-                Rule::excludeIf(! in_array($inPerson, $this->input('meeting_types', []))),
-                Rule::requiredIf(in_array($inPerson, $this->input('meeting_types', []))),
+                Rule::excludeIf(! in_array($inPerson, is_array($this->input('meeting_types')) ? $this->input('meeting_types') : [])),
+                Rule::requiredIf(in_array($inPerson, is_array($this->input('meeting_types')) ? $this->input('meeting_types') : [])),
                 new Enum(ProvinceOrTerritory::class),
             ],
             'postal_code' => [
                 'nullable',
-                Rule::excludeIf(! in_array($inPerson, $this->input('meeting_types', []))),
-                Rule::requiredIf(in_array($inPerson, $this->input('meeting_types', []))),
+                Rule::excludeIf(! in_array($inPerson, is_array($this->input('meeting_types')) ? $this->input('meeting_types') : [])),
+                Rule::requiredIf(in_array($inPerson, is_array($this->input('meeting_types')) ? $this->input('meeting_types') : [])),
                 'postal_code:CA',
             ],
             'directions' => [
                 'nullable',
-                Rule::excludeIf(! in_array($inPerson, $this->input('meeting_types', []))),
+                Rule::excludeIf(! in_array($inPerson, is_array($this->input('meeting_types')) ? $this->input('meeting_types') : [])),
                 'array',
             ],
             'meeting_software' => [
                 'nullable',
-                Rule::excludeIf(! in_array($webConference, $this->input('meeting_types', []))),
-                Rule::requiredIf(in_array($webConference, $this->input('meeting_types', []))),
+                Rule::excludeIf(! in_array($webConference, is_array($this->input('meeting_types')) ? $this->input('meeting_types') : [])),
+                Rule::requiredIf(in_array($webConference, is_array($this->input('meeting_types')) ? $this->input('meeting_types') : [])),
+                'string',
             ],
             'alternative_meeting_software' => [
                 'nullable',
-                Rule::excludeIf(! in_array($webConference, $this->input('meeting_types', []))),
+                Rule::excludeIf(! in_array($webConference, is_array($this->input('meeting_types')) ? $this->input('meeting_types') : [])),
                 'boolean',
             ],
             'meeting_url' => [
                 'nullable',
-                Rule::excludeIf(! in_array($webConference, $this->input('meeting_types', []))),
-                Rule::requiredIf(in_array($webConference, $this->input('meeting_types', []))),
+                Rule::excludeIf(! in_array($webConference, is_array($this->input('meeting_types')) ? $this->input('meeting_types') : [])),
+                Rule::requiredIf(in_array($webConference, is_array($this->input('meeting_types')) ? $this->input('meeting_types') : [])),
+                'url',
             ],
             'additional_video_information' => [
                 'nullable',
-                Rule::excludeIf(! in_array($webConference, $this->input('meeting_types', []))),
+                Rule::excludeIf(! in_array($webConference, is_array($this->input('meeting_types')) ? $this->input('meeting_types') : [])),
                 'array',
             ],
             'meeting_phone' => [
                 'nullable',
-                Rule::excludeIf(! in_array($phone, $this->input('meeting_types', []))),
-                Rule::requiredIf(in_array($phone, $this->input('meeting_types', []))),
-                'phone:CA',
+                Rule::excludeIf(! in_array($phone, is_array($this->input('meeting_types')) ? $this->input('meeting_types') : [])),
+                Rule::requiredIf(in_array($phone, is_array($this->input('meeting_types')) ? $this->input('meeting_types') : [])),
+                'phone:CA,US',
             ],
             'additional_phone_information' => [
                 'nullable',
-                Rule::excludeIf(! in_array($phone, $this->input('meeting_types', []))),
+                Rule::excludeIf(! in_array($phone, is_array($this->input('meeting_types')) ? $this->input('meeting_types') : [])),
                 'array',
             ],
         ];
@@ -109,23 +114,33 @@ public function attributes(): array
             'end_time' => __('meeting end time'),
             'date' => __('meeting date'),
             'timezone' => __('meeting time zone'),
+            'locality' => __('city or town'),
             'region' => __('province or territory'),
             'meeting_url' => __('link to join the meeting'),
-            'meeting_phone' => __('meeting phone number'),
+            'meeting_phone' => __('phone number to join the meeting'),
         ];
     }
 
     public function messages(): array
     {
         return [
-            'title.*.required_without' => __('A meeting title must be provided in at least one language.'),
+            'date.required' => __('You must enter a :attribute'),
+            'title.*.required_without' => __('A meeting title must be provided in at least English or French.'),
             'meeting_types.required' => __('You must indicate at least one way for participants to attend the meeting.'),
+            'meeting_types.*.Illuminate\Validation\Rules\Enum' => __('You must select a valid meeting type.'),
             'street_address.required' => __('You must enter a :attribute for the meeting location.'),
             'locality.required' => __('You must enter a :attribute for the meeting location.'),
             'region.required' => __('You must enter a :attribute for the meeting location.'),
             'postal_code.required' => __('You must enter a :attribute for the meeting location.'),
             'meeting_software.required' => __('You must indicate the :attribute.'),
-            'meeting_url.required' => __('You must provide a :attribute.'),
+            'meeting_url.required' => __('You must enter a :attribute.'),
+            'meeting_phone.required' => __('You must enter a :attribute.'),
+            'start_time.required' => __('You must enter a :attribute'),
+            'start_time.date_format' => __('The :attribute format is not valid.'),
+            'start_time.before' => __('The :attribute must be before the :date.'),
+            'end_time.required' => __('You must enter a :attribute'),
+            'end_time.date_format' => __('The :attribute format is not valid.'),
+            'end_time.after' => __('The :attribute must be after the :date.'),
         ];
     }
 }