fix: validation error with only other for outcome analysis (resolves #…

…2057) (#2104) * fix: validation error with only other for outcome analysis * test: add missing test case * test: add coverage for UpdateProjectTeamRequest * test: add coverage for DestroyProjectRequest * test: add coverage for StoreProjectRequest * test: clean up project test * test: add coverage for StoreProjectLanguagesRequest * test: add coverage for StoreProjectContextRequest * fix: publishable check failing with outcome analysis other
accessibility-exchange · Jan 18, 2024 · 0458cf9 · 0458cf9
1 parent b4f8c3f
commit 0458cf9
Show file tree

Hide file tree

Showing 20 changed files with 692 additions and 71 deletions.
diff --git a/app/Http/Controllers/ProjectController.php b/app/Http/Controllers/ProjectController.php
@@ -133,6 +133,10 @@ public function update(UpdateProjectRequest $request, Project $project): Redirec
     {
         $data = $request->validated();
 
+        if (empty($data['has_other_outcome_analysis'])) {
+            $data['outcome_analysis_other'] = [];
+        }
+
         $project->fill($data);
         $project->save();
 

diff --git a/app/Http/Requests/StoreProjectContextRequest.php b/app/Http/Requests/StoreProjectContextRequest.php
@@ -15,7 +15,7 @@ public function rules(): array
     {
         return [
             'context' => 'required|string|in:new,follow-up',
-            'ancestor' => 'nullable|integer|required_if:context,follow-up,exists:projects,id',
+            'ancestor' => 'nullable|integer|required_if:context,follow-up|exists:projects,id',
         ];
     }
 

diff --git a/app/Http/Requests/StoreProjectRequest.php b/app/Http/Requests/StoreProjectRequest.php
@@ -4,9 +4,12 @@
 
 use Illuminate\Foundation\Http\FormRequest;
 use Illuminate\Validation\Rule;
+use Worksome\RequestFactories\Concerns\HasFactory;
 
 class StoreProjectRequest extends FormRequest
 {
+    use HasFactory;
+
     /**
      * Determine if the user is authorized to make this request.
      *
@@ -31,16 +34,16 @@ public function rules()
             'projectable_id' => [
                 'required',
                 'integer',
-                Rule::in($projectable_type::pluck('id')->toArray()),
+                Rule::in(class_exists($projectable_type) ? $projectable_type::pluck('id')->toArray() : []),
             ],
             'ancestor_id' => [
                 'nullable',
                 'integer',
                 'exists:App\Models\Project,id',
             ],
-            'name.*' => 'nullable|string|max:255|unique_translation:projects',
             'name.en' => 'required_without:name.fr',
             'name.fr' => 'required_without:name.en',
+            'name.*' => 'nullable|string|max:255|unique_translation:projects',
         ];
     }
 
@@ -66,8 +69,6 @@ public function messages()
         return [
             'name.*.unique_translation' => __('A project with this name already exists.'),
             'name.*.required_without' => __('A project name must be provided in at least one language.'),
-            'goals.*.required_without' => __('Project goals must be provided in at least one language.'),
-            'scope.*.required_without' => __('Project scope must be provided in at least one language.'),
         ];
     }
 }
diff --git a/app/Http/Requests/UpdateProjectRequest.php b/app/Http/Requests/UpdateProjectRequest.php
@@ -8,9 +8,12 @@
 use Illuminate\Foundation\Http\FormRequest;
 use Illuminate\Validation\Rules\Enum;
 use Illuminate\Validation\Validator;
+use Worksome\RequestFactories\Concerns\HasFactory;
 
 class UpdateProjectRequest extends FormRequest
 {
+    use HasFactory;
+
     public function authorize(): bool
     {
         return $this->user()->can('update', $this->project);
@@ -19,21 +22,20 @@ public function authorize(): bool
     public function rules(): array
     {
         return [
-
+            'name.en' => 'required_without:name.fr',
+            'name.fr' => 'required_without:name.en',
             'name.*' => [
                 'nullable',
                 'string',
                 'max:255',
                 UniqueTranslationRule::for('projects')->ignore($this->project->id),
             ],
-            'name.en' => 'required_without:name.fr',
-            'name.fr' => 'required_without:name.en',
-            'goals.*' => 'nullable|string',
             'goals.en' => 'required_without:goals.fr',
             'goals.fr' => 'required_without:goals.en',
-            'scope.*' => 'nullable|string',
+            'goals.*' => 'nullable|string',
             'scope.en' => 'required_without:scope.fr',
             'scope.fr' => 'required_without:scope.en',
+            'scope.*' => 'nullable|string',
             'regions' => 'required|array',
             'regions.*' => [
                 'nullable',
@@ -45,13 +47,14 @@ public function rules(): array
             'out_of_scope.*' => 'nullable|string',
             'start_date' => 'required|date|before:end_date',
             'end_date' => 'required|date|after:start_date',
-            'outcome_analysis' => 'required|array',
+            'outcome_analysis' => 'required_without:has_other_outcome_analysis|array',
             'outcome_analysis.*' => 'string|in:internal,external',
-            'outcome_analysis_other' => 'nullable|array',
+            'has_other_outcome_analysis' => 'required_without:outcome_analysis|boolean',
+            'outcome_analysis_other' => 'nullable|array|exclude_if:has_other_outcome_analysis,false|exclude_unless:has_other_outcome_analysis,true',
             'outcome_analysis_other.*' => 'nullable|string',
-            'outcomes.*' => 'nullable|string',
             'outcomes.en' => 'required_without:outcomes.fr',
             'outcomes.fr' => 'required_without:outcomes.en',
+            'outcomes.*' => 'nullable|string',
             'public_outcomes' => 'required|boolean',
         ];
     }
@@ -78,7 +81,10 @@ public function attributes(): array
             'end_date' => __('end date'),
             'outcome_analysis' => __('Outcomes and reports'),
             'outcome_analysis.*' => __('Outcomes and reports'),
+            'has_other_outcome_analysis' => __('Outcomes and reports other'),
             'outcome_analysis_other' => __('Outcomes and reports other'),
+            'outcome_analysis_other.en' => __('Outcomes and reports other (English)'),
+            'outcome_analysis_other.fr' => __('Outcomes and reports other (French)'),
             'outcome_analysis_other.*' => __('Outcomes and reports other'),
             'outcomes.en' => __('Project outcome (English)'),
             'outcomes.fr' => __('Project outcome (French)'),
@@ -94,7 +100,9 @@ public function messages(): array
             'name.*.required_without' => __('A project name must be provided in at least one language.'),
             'goals.*.required_without' => __('Project goals must be provided in at least one language.'),
             'scope.*.required_without' => __('Project scope must be provided in at least one language.'),
-            'outcome_analysis.required' => __('You must identify who will be going through the results and producing an outcome.'),
+            'outcome_analysis.required_without' => __('You must identify who will be going through the results and producing an outcome.'),
+            'has_other_outcome_analysis.required_without' => __('You must identify who will be going through the results and producing an outcome.'),
+            'outcome_analysis_other.*.required_without' => __('You must identify the other team that will be going through the results and producing an outcome.'),
             'outcomes.*.required_without' => __('Tangible outcomes must be provided in at least one language.'),
             'public_outcomes.required' => __('You must indicate if the reports will be publicly available.'),
         ];
@@ -106,23 +114,34 @@ public function prepareForValidation()
             'impacts' => [],
             'regions' => [],
             'outcome_analysis' => [],
-            'other' => 0,
         ];
 
         // Prepare input for validation
         $this->mergeIfMissing($fallbacks);
-        if (! $this['other']) {
-            $this->merge(['outcome_analysis_other' => []]);
-        }
 
         // Prepare old input in case of validation failure
         request()->mergeIfMissing($fallbacks);
+
+        // Prevents the model values from coming back if the other outcome analysis options have been removed when a
+        // validation error occurs.
+        request()->mergeIfMissing([
+            'outcome_analysis_other' => [],
+            'has_other_outcome_analysis' => 0,
+        ]);
     }
 
     public function withValidator(Validator $validator)
     {
         $validator->sometimes('impacts', 'required', function ($input) {
             return $this->project->projectable instanceof RegulatedOrganization;
         });
+
+        $validator->sometimes('outcome_analysis_other.en', 'required_without:outcome_analysis_other.fr', function ($input) {
+            return ! empty($input->has_other_outcome_analysis);
+        });
+
+        $validator->sometimes('outcome_analysis_other.fr', 'required_without:outcome_analysis_other.en', function ($input) {
+            return ! empty($input->has_other_outcome_analysis);
+        });
     }
 }
diff --git a/app/Http/Requests/UpdateProjectTeamRequest.php b/app/Http/Requests/UpdateProjectTeamRequest.php
@@ -3,9 +3,12 @@
 namespace App\Http\Requests;
 
 use Illuminate\Foundation\Http\FormRequest;
+use Worksome\RequestFactories\Concerns\HasFactory;
 
 class UpdateProjectTeamRequest extends FormRequest
 {
+    use HasFactory;
+
     public function authorize(): bool
     {
         return true;
@@ -89,7 +92,7 @@ public function prepareForValidation()
                     $training['trainer_url'] = normalize_url($training['trainer_url']);
 
                     return $training;
-                }, $this->team_trainings ?? []),
+                }, is_array($this->team_trainings) ? $this->team_trainings : []),
             ]);
 
         // Prepare old input in case of validation failure

diff --git a/app/Models/Project.php b/app/Models/Project.php
@@ -214,7 +214,7 @@ public function isPreviewable(): bool
             'languages' => 'required',
             'name.en' => 'required_without:name.fr',
             'name.fr' => 'required_without:name.en',
-            'outcome_analysis' => 'required',
+            'outcome_analysis' => 'required_without:outcome_analysis_other',
             'preferred_contact_method' => 'required',
             'projectable_id' => 'required',
             'projectable_type' => 'required',

diff --git a/resources/lang/en.json b/resources/lang/en.json
@@ -1091,6 +1091,8 @@
     "Our team has people with lived and living experiences of disability or being Deaf.": "Our team has people with lived and living experiences of disability or being Deaf.",
     "Outcomes and reports": "Outcomes and reports",
     "Outcomes and reports other": "Outcomes and reports other",
+    "Outcomes and reports other (English)": "Outcomes and reports other (English)",
+    "Outcomes and reports other (French)": "Outcomes and reports other (French)",
     "out of scope": "out of scope",
     "Pacific Standard or Daylight Time": "Pacific Standard or Daylight Time",
     "page": "page",
@@ -2004,6 +2006,7 @@
     "You must enter your organization’s name in either English or French.": "You must enter your organization’s name in either English or French.",
     "You must fill out the field “About your organization”.": "You must fill out the field “About your organization”.",
     "You must fill out your [payment information](:url) before you can sign up.": "You must fill out your [payment information](:url) before you can sign up.",
+    "You must identify the other team that will be going through the results and producing an outcome.": "You must identify the other team that will be going through the results and producing an outcome.",
     "You must identify who will be going through the results and producing an outcome.": "You must identify who will be going through the results and producing an outcome.",
     "You must indicate at least one way for participants to attend the meeting.": "You must indicate at least one way for participants to attend the meeting.",
     "You must indicate if the reports will be publicly available.": "You must indicate if the reports will be publicly available.",

diff --git a/resources/lang/fr.json b/resources/lang/fr.json
@@ -1091,6 +1091,8 @@
     "Our team has people with lived and living experiences of disability or being Deaf.": "Notre équipe compte des personnes en situation de handicap ou des personnes sourdes, ou des personnes ayant une expérience vécue du handicap ou de la surdité.",
     "Outcomes and reports": "Résultats et rapports",
     "Outcomes and reports other": "Outcomes and reports other",
+    "Outcomes and reports other (English)": "",
+    "Outcomes and reports other (French)": "",
     "out of scope": "out of scope",
     "Pacific Standard or Daylight Time": "Heure normale ou avancée du Pacifique",
     "page": "page",
@@ -2004,6 +2006,7 @@
     "You must enter your organization’s name in either English or French.": "Vous devez saisir le nom de votre organisation en anglais ou en français.",
     "You must fill out the field “About your organization”.": "Vous devez remplir le champ « À propos de votre organisation ».",
     "You must fill out your [payment information](:url) before you can sign up.": "Vous devez rentrer vos [informations de paiement](:url) avant de pouvoir vous joindre.",
+    "You must identify the other team that will be going through the results and producing an outcome.": "",
     "You must identify who will be going through the results and producing an outcome.": "Vous devez identifier qui examinera les résultats et produira un rapport.",
     "You must indicate at least one way for participants to attend the meeting.": "Vous devez indiquer au moins un moyen pour les personnes participantes de se joindre à la réunion.",
     "You must indicate if the reports will be publicly available.": "Veuillez indiquer si les rapports seront accessibles au public.",

diff --git a/resources/views/projects/edit/1.blade.php b/resources/views/projects/edit/1.blade.php
@@ -74,17 +74,18 @@
             <h3>{{ __('Project outcome') }}</h3>
             <x-interpretation name="{{ __('Project outcome', [], 'en') }}" />
 
-            <fieldset class="field @error('outcome_analysis') field--error @enderror stack" x-data="{ otherOutcomeAnalysis: {{ old('other', !is_null($project->outcome_analysis_other) && $project->outcome_analysis_other !== '' ? 'true' : 'false') }} }">
+            <fieldset class="field @error('outcome_analysis') field--error @enderror stack" x-data="{ otherOutcomeAnalysis: {{ old('has_other_outcome_analysis', !is_null($project->outcome_analysis_other) && $project->outcome_analysis_other !== '' ? 'true' : 'false') }} }">
                 <legend>
                     {{ __('Who will be going through the results and producing an outcome?') . ' ' . __('(required)') }}
                 </legend>
                 <x-hearth-checkboxes name="outcome_analysis" :options="\Spatie\LaravelOptions\Options::forEnum(App\Enums\OutcomeAnalyzer::class)->toArray()" :checked="old('outcome_analysis', $project->outcome_analysis ?? [])" required />
                 <div class="field">
-                    <x-hearth-checkbox name="other" :checked="old(
-                        'other',
+                    <x-hearth-checkbox name="has_other_outcome_analysis" :checked="old(
+                        'has_other_outcome_analysis',
                         !is_null($project->outcome_analysis_other) && $project->outcome_analysis_other !== '',
-                    )" x-model="otherOutcomeAnalysis" />
-                    <x-hearth-label for='other'>{{ __('Other') }}</x-hearth-label>
+                    )"
+                        x-model="otherOutcomeAnalysis" />
+                    <x-hearth-label for='has_other_outcome_analysis'>{{ __('Other') }}</x-hearth-label>
                 </div>
                 <div class="field__subfield stack">
                     <x-translatable-input name="outcome_analysis_other" :label="__('Please indicate who will be going through the results')" :short-label="__('who is going through the results')"

diff --git a/tests/Datasets/DestroyProjectRequestValidationErrors.php b/tests/Datasets/DestroyProjectRequestValidationErrors.php
@@ -0,0 +1,18 @@
+<?php
+
+dataset('destroyProjectRequestValidationErrors', function () {
+    return [
+        'Current password is missing' => [
+            'state' => [],
+            'errors' => fn () => ['current_password' => __('validation.required', ['attribute' => __('current password')])],
+        ],
+        'Current password is not a string' => [
+            'state' => ['current_password' => false],
+            'errors' => fn () => ['current_password' => __('validation.string', ['attribute' => __('current password')])],
+        ],
+        'Current password is not valid' => [
+            'state' => ['current_password' => 'WrongPassword'],
+            'errors' => fn () => ['current_password' => __('The provided password does not match your current password.')],
+        ],
+    ];
+});
diff --git a/tests/Datasets/ProjectIsPublishable.php b/tests/Datasets/ProjectIsPublishable.php
@@ -72,7 +72,10 @@
         ],
         'not publishable when missing outcome_analysis' => [
             false,
-            array_replace_recursive($baseModel, ['outcome_analysis' => null]),
+            array_replace_recursive($baseModel, [
+                'outcome_analysis' => null,
+                'outcome_analysis_other' => [],
+            ]),
         ],
         'not publishable when missing team_trainings.*.date' => [
             false,

diff --git a/tests/Datasets/StoreProjectContextRequestValidationErrors.php b/tests/Datasets/StoreProjectContextRequestValidationErrors.php
@@ -0,0 +1,30 @@
+<?php
+
+dataset('storeProjectContextRequestValidationErrors', function () {
+    return [
+        'Context type is missing' => [
+            'state' => ['context' => null],
+            'errors' => fn () => ['context' => __('validation.required', ['attribute' => __('project context')])],
+        ],
+        'Context is not a string' => [
+            'state' => ['context' => false],
+            'errors' => fn () => ['context' => __('validation.string', ['attribute' => __('project context')])],
+        ],
+        'Context is not valid' => [
+            'state' => ['context' => 'old'],
+            'errors' => fn () => ['context' => __('validation.exists', ['attribute' => __('project context')])],
+        ],
+        'Ancestor is not an integer' => [
+            'state' => ['ancestor' => false, 'context' => 'new'],
+            'errors' => fn () => ['ancestor' => __('validation.integer', ['attribute' => __('previous project')])],
+        ],
+        'Ancestor is missing' => [
+            'state' => ['ancestor' => null, 'context' => 'follow-up'],
+            'errors' => fn () => ['ancestor' => __('Since this is a follow-up to a previous project, you must specify the previous project.')],
+        ],
+        'Ancestor is invalid' => [
+            'state' => ['ancestor' => 1000000, 'context' => 'new'],
+            'errors' => fn () => ['ancestor' => __('validation.exists', ['attribute' => __('previous project')])],
+        ],
+    ];
+});
diff --git a/tests/Datasets/StoreProjectLanguagesRequestValidationErrors.php b/tests/Datasets/StoreProjectLanguagesRequestValidationErrors.php
@@ -0,0 +1,18 @@
+<?php
+
+dataset('storeProjectLanguagesRequestValidationErrors', function () {
+    return [
+        'Languages type is missing' => [
+            'state' => ['languages' => null],
+            'errors' => fn () => ['languages' => __('validation.required', ['attribute' => __('project languages')])],
+        ],
+        'Languages is not an array' => [
+            'state' => ['languages' => false],
+            'errors' => fn () => ['languages' => __('validation.array', ['attribute' => __('project languages')])],
+        ],
+        'Languages array is empty' => [
+            'state' => ['languages' => []],
+            'errors' => fn () => ['languages' => __('validation.required', ['attribute' => __('project languages')])],
+        ],
+    ];
+});