Fix dynamic upload file field required indicator + make option naming…

… consistent (#10497)

* Fix file field required indicator + consistency on option naming

- Fix missing required file indicator from the dynamic upload file
  field label
- Refactor the `optional` option for the file field and upload
  modal to `required` (inverse option) to maintain concistency
  over the option naming for all form fields

* Fix a11y issue with the label `for` attribute in the upload label

decidim-admin/app/views/decidim/admin/attachments/_form.html.erb

@@ -23,7 +23,7 @@
     </div>
 
     <div class="row column">
-      <%= form.upload :file, optional: false %>
+      <%= form.upload :file, required: true %>
     </div>
   </div>
 </div>

decidim-admin/app/views/decidim/admin/imports/new.html.erb

@@ -38,7 +38,7 @@
               </div>
             </legend>
             <div class="row column">
-              <%= form.upload :file, optional: false, help_i18n_scope: "decidim.admin.forms.file_help.import" %>
+              <%= form.upload :file, required: true, help_i18n_scope: "decidim.admin.forms.file_help.import" %>
             </div>
           </fieldset>
         </div>

decidim-admin/app/views/decidim/admin/participatory_space_private_users_csv_imports/new.html.erb

@@ -27,7 +27,7 @@
     <%= decidim_form_for(@form, url: participatory_space_private_users_csv_imports_path, html: { class: "form" }) do |form| %>
       <p><%= t(".explanation") %></p>
       <div class="row column">
-        <%= form.upload :file, optional: false %>
+        <%= form.upload :file, required: true %>
       </div>
 
       <div class="button--double form-general-submit">

decidim-admin/app/views/decidim/admin/user_groups_csv_verifications/new.html.erb

@@ -8,7 +8,7 @@
     <%= decidim_form_for(@form, url: user_groups_csv_verification_path, html: { class: "form" }) do |form| %>
       <p><%= t(".explanation") %></p>
       <div class="row column">
-        <%= form.upload :file, optional: false %>
+        <%= form.upload :file, required: true %>
       </div>
 
       <div class="button--double form-general-submit">

decidim-assemblies/app/views/decidim/assemblies/admin/assembly_imports/_form.html.erb

@@ -18,7 +18,7 @@
           <fieldset>
             <legend><%= t(".document_legend") %> </legend>
               <div class="row column">
-                <%= form.upload :document, optional: false %>
+                <%= form.upload :document, required: true %>
               </div>
           </fieldset>
         </div>

decidim-conferences/app/views/decidim/conferences/admin/diplomas/_form.html.erb

@@ -11,13 +11,13 @@
   <div class="card-section">
     <div class="row">
       <div class="columns xlarge-6">
-        <%= form.upload :main_logo, optional: false %>
+        <%= form.upload :main_logo, required: true %>
       </div>
     </div>
 
     <div class="row">
       <div class="columns xlarge-6">
-        <%= form.upload :signature, optional: false %>
+        <%= form.upload :signature, required: true %>
       </div>
     </div>
     <div class="row column">

decidim-conferences/app/views/decidim/conferences/admin/partners/_form.html.erb

@@ -24,7 +24,7 @@
     </div>
 
     <div class="row column">
-      <%= form.upload :logo, optional: false %>
+      <%= form.upload :logo, required: true %>
     </div>
   </div>
 </div>

decidim-core/app/cells/decidim/upload_modal/files.erb

@@ -30,6 +30,7 @@
           add_attribute: add_attribute,
           resource_name: resource_name,
           resource_class: resource_class,
+          required: required?,
           optional: optional,
           max_file_size: max_file_size,
           multiple: multiple,

decidim-core/app/cells/decidim/upload_modal_cell.rb

@@ -29,7 +29,7 @@ def button_class
     end
 
     def label
-      options[:label]
+      form.send(:custom_label, attribute, options[:label], { required: required?, for: nil })
     end
 
     def button_label
@@ -72,16 +72,25 @@ def multiple
       options[:multiple] || false
     end
 
+    # @deprecated Please use `required?` instead.
+    #
+    # NOTE: When this is removed, also the `optional` option should be removed.
     def optional
-      options[:optional]
+      !required?
+    end
+
+    def required?
+      return !options[:optional] if options.has_key?(:optional)
+
+      options[:required] == true
     end
 
     # By default Foundation adds form errors next to input, but since input is in the modal
     # and modal is hidden by default, we need to add an additional validation field to the form.
     # This should only be necessary when file is required by the form.
     def input_validation_field
       object_name = form.object.present? ? "#{form.object.model_name.param_key}[#{add_attribute}_validation]" : "#{add_attribute}_validation"
-      input = check_box_tag object_name, 1, attachments.present?, class: "hide", label: false, required: !optional
+      input = check_box_tag object_name, 1, attachments.present?, class: "hide", label: false, required: required?
       message = form.send(:abide_error_element, add_attribute) + form.send(:error_and_help_text, add_attribute)
       input + message
     end

decidim-core/app/packs/src/decidim/direct_uploads/upload_modal.js

@@ -16,7 +16,6 @@ export default class UploadModal {
       // - addAttribute - Field name / attribute of resource (e.g. avatar)
       // - resourceName - The resource to which the attribute belongs (e.g. user)
       // - resourceClass - Ruby class of the resource (e.g. Decidim::User)
-      // - optional - Defines if file is optional
       // - multiple - Defines if multiple files can be uploaded
       // - titled - Defines if file(s) can have titles
       // - maxFileSize - Defines maximum file size in bytes

decidim-core/lib/decidim/form_builder.rb

@@ -444,7 +444,7 @@ def attachment(attribute, options = {})
     #              * resouce_name: Name of the resource (e.g. user)
     #              * resource_class: Attribute's resource class (e.g. Decidim::User)
     #              * resouce_class: Class of the resource (e.g. user)
-    #              * optional: Whether the file can be optional or not.
+    #              * required: Whether the file is required or not (false by default).
     #              * titled: Whether the file can have title or not.
     #              * show_current: Whether the current file is displayed next to the button.
     #              * help: Array of help messages which are displayed inside of the upload modal.
@@ -464,7 +464,7 @@ def upload(attribute, options = {})
         attribute: attribute,
         resource_name: @object_name,
         resource_class: options[:resource_class]&.to_s || resource_class(attribute),
-        optional: true,
+        required: false,
         titled: false,
         show_current: true,
         max_file_size: max_file_size,
@@ -712,6 +712,8 @@ def custom_label(attribute, text, options, field_before_label: false, show_requi
                safe_join([yield, text.html_safe])
              elsif block_given?
                safe_join([text.html_safe, yield])
+             else
+               text
              end
 
       label(attribute, text, options || {})

decidim-core/spec/cells/decidim/upload_modal_cell_spec.rb

@@ -6,17 +6,18 @@
   subject { my_cell.call }
 
   let(:my_cell) { cell("decidim/upload_modal", form, options) }
-  let(:form) do
-    double(
-      object: object,
-      object_name: "object",
-      file_field: file_field,
-      abide_error_element: "",
-      error_and_help_text: ""
-    )
-  end
+  let(:form) { Decidim::FormBuilder.new(:object, object, view, {}) }
+  let(:view) { Class.new(ActionView::Base).new(ActionView::LookupContext.new(ActionController::Base.view_paths), {}, []) }
   let(:object) do
-    double
+    Class.new do
+      def self.model_name
+        ActiveModel::Name.new(self, nil, "dummy")
+      end
+
+      def model_name
+        self.class.model_name
+      end
+    end.new
   end
   let(:file_field) { double }
   let(:file_validation_humanizer) do
@@ -29,14 +30,14 @@
       attribute: attribute,
       resource_name: resource_name,
       attachments: attachments,
-      optional: optional,
+      required: required,
       titled: titled
     }
   end
   let(:attribute) { "dummy_attribute" }
   let(:resource_name) { "dummy" }
   let(:attachments) { [] }
-  let(:optional) { true }
+  let(:required) { false }
   let(:titled) { false }
 
   before do
@@ -56,18 +57,35 @@
   end
 
   context "when file is required" do
-    let(:optional) { false }
-    let(:object) do
-      double(
-        model_name: double(
-          param_key: param_key
-        )
-      )
+    let(:required) { true }
+
+    it "renders hidden checkbox" do
+      expect(subject).to have_css("input[name='dummy[#{attribute}_validation]']")
+    end
+
+    it "renders the required field indicator" do
+      expect(subject).to have_css("label .label-required", text: "Required field")
+    end
+  end
+
+  # @deprecated Remove after removing the `optional` option.
+  context "when file is not optional" do
+    let(:options) do
+      {
+        attribute: attribute,
+        resource_name: resource_name,
+        attachments: attachments,
+        optional: false,
+        titled: titled
+      }
     end
-    let(:param_key) { "dummy_param_key" }
 
     it "renders hidden checkbox" do
-      expect(subject).to have_css("input[name='#{param_key}[#{attribute}_validation]']")
+      expect(subject).to have_css("input[name='dummy[#{attribute}_validation]']")
+    end
+
+    it "renders the required field indicator" do
+      expect(subject).to have_css("label .label-required", text: "Required field")
     end
   end
 

decidim-elections/app/views/decidim/votings/census/admin/census/_new_census.html.erb

@@ -12,7 +12,7 @@
                             method: :post,
                             html: { class: "form new_census" }) do |form| %>
         <div class="field">
-          <%= form.upload :file, optional: false, help_i18n_scope: "decidim.votings.census.admin.census.new.file_help" %>
+          <%= form.upload :file, required: true, help_i18n_scope: "decidim.votings.census.admin.census.new.file_help" %>
         </div>
 
         <%= submit_tag t("submit", scope: "decidim.votings.census.admin.census.new"), class: "button" %>

decidim-initiatives/app/views/decidim/initiatives/admin/initiatives_types/_form.html.erb

@@ -14,7 +14,7 @@
 
     <div class="row">
       <div class="columns xlarge-6">
-        <%= form.upload :banner_image, optional: false %>
+        <%= form.upload :banner_image, required: true %>
       </div>
     </div>
   </div>

decidim-participatory_processes/app/views/decidim/participatory_processes/admin/participatory_process_imports/_form.html.erb

@@ -16,7 +16,7 @@
           <fieldset>
             <legend><%= t(".document_legend") %> </legend>
               <div class="row column">
-                <%= form.upload :document, optional: false %>
+                <%= form.upload :document, required: true %>
               </div>
           </fieldset>
         </div>

decidim-proposals/app/views/decidim/proposals/admin/participatory_texts/new_import.html.erb

@@ -23,7 +23,7 @@
           <fieldset>
             <legend> <%= t(".document_legend", valid_mime_types: mime_types_with_document_examples).html_safe %> </legend>
               <div class="row column">
-                <%= form.upload :document, optional: false %>
+                <%= form.upload :document, required: true %>
               </div>
           </fieldset>
         </div>

decidim-system/app/views/decidim/system/oauth_applications/_form.html.erb

@@ -22,5 +22,5 @@
 </div>
 
 <div class="row column">
-  <%= form.upload :organization_logo, optional: false %>
+  <%= form.upload :organization_logo, required: true %>
 </div>