Fix question form errors not being displayed

CHANGELOG.md

@@ -39,5 +39,6 @@ controller or added a new module you need to rename `feature` to `component`.
 - **decidim-surveys**: Fix errored questions being re-rendered with disabled inputs [\#3014](https://github.com/decidim/decidim/pull/3014)
 - **decidim-surveys**: Fix errored questions rendering answer options as empty fields [\#3014](https://github.com/decidim/decidim/pull/3014)
 - **decidim-surveys**: Fix translated fields of freshly created questions not working after form errors [\#3026](https://github.com/decidim/decidim/pull/3026)
+- **decidim-surveys**: Fix question form errors not being displayed [\#3046](https://github.com/decidim/decidim/pull/3046)
 
 Please check [0.10-stable](https://github.com/decidim/decidim/blob/0.10-stable/CHANGELOG.md) for previous changes.

decidim-surveys/app/assets/javascripts/decidim/surveys/admin/dynamic_fields.component.js.es6

@@ -1,7 +1,6 @@
 ((exports) => {
   class DynamicFieldsComponent {
     constructor(options = {}) {
-      this.templateId = options.templateId;
       this.wrapperSelector = options.wrapperSelector;
       this.containerSelector = options.containerSelector;
       this.fieldSelector = options.fieldSelector;
@@ -13,34 +12,35 @@
       this.onRemoveField = options.onRemoveField;
       this.onMoveUpField = options.onMoveUpField;
       this.onMoveDownField = options.onMoveDownField;
-      this.tabsPrefix = options.tabsPrefix;
+      this.placeholderId = options.placeholderId;
+      this.elementCounter = 0;
       this._enableInterpolation();
       this._activateFields();
       this._bindEvents();
     }
 
     _enableInterpolation() {
-      $.fn.template = function(placeholder, value) {
-        $(this).find(`[id^=${placeholder}]`).each((index, element) => {
-          $(element).attr("id", $(element).attr("id").replace(placeholder, value));
-        });
-
-        $(this).find(`[data-tabs-content=${placeholder}]`).each((index, element) => {
-          $(element).attr("data-tabs-content", value);
+      $.fn.replaceAttribute = function(attribute, placeholder, value) {
+        $(this).find(`[${attribute}*=${placeholder}]`).each((index, element) => {
+          $(element).attr(attribute, $(element).attr(attribute).replace(placeholder, value));
         });
 
-        $(this).find(`[for^=${placeholder}]`).each((index, element) => {
-          $(element).attr("for", $(element).attr("for").replace(placeholder, value));
-        });
+        return this;
+      }
 
-        $(this).find(`[tabs_id=${placeholder}]`).each((index, element) => {
-          $(element).attr("tabs_id", value);
-        });
+      $.fn.template = function(placeholder, value) {
+        const $subtemplate = $(this).find("template");
 
+        if ($subtemplate.length > 0) {
+          $subtemplate.html((index, oldHtml) => $(oldHtml).template(placeholder, value)[0].outerHTML);
+        }
 
-        $(this).find(`[href^='#${placeholder}']`).each((index, element) => {
-          $(element).attr("href", $(element).attr("href").replace(placeholder, value));
-        });
+        $(this).replaceAttribute("id", placeholder, value);
+        $(this).replaceAttribute("name", placeholder, value);
+        $(this).replaceAttribute("data-tabs-content", placeholder, value);
+        $(this).replaceAttribute("for", placeholder, value);
+        $(this).replaceAttribute("tabs_id", placeholder, value);
+        $(this).replaceAttribute("href", placeholder, value);
 
         return this;
       }
@@ -82,7 +82,8 @@
 
     _addField() {
       const $container = $(this.wrapperSelector).find(this.containerSelector);
-      const $newField = $($(`#${this.templateId}`).html()).template(this._getPlaceholderTabId(), this._getUniqueTabId());
+      const $template = $(this.wrapperSelector).children("template");
+      const $newField = $($template.html()).template(this.placeholderId, this._getUID());
 
       $newField.find('ul.tabs').attr('data-tabs', true);
 
@@ -141,22 +142,16 @@
 
     _activateFields() {
       $(this.fieldSelector).each((idx, el) => {
-        $(el).template(this._getPlaceholderTabId(), this._getUniqueTabId());
+        $(el).template(this.placeholderId, this._getUID());
 
         $(el).find('ul.tabs').attr('data-tabs', true);
       })
     }
 
-    _getPlaceholderTabId() {
-      return `${this.tabsPrefix}-id`;
-    }
-
-    _getUniqueTabId() {
-      return `${this.tabsPrefix}-${this._getUID()}`;
-    }
-
     _getUID() {
-      return `${new Date().getTime()}-${Math.floor(Math.random() * 1000000)}`;
+      this.elementCounter += 1;
+
+      return (new Date().getTime()) + this.elementCounter;
     }
   }
 

decidim-surveys/app/assets/javascripts/decidim/surveys/admin/surveys.js.es6

@@ -7,14 +7,14 @@
 
   const wrapperSelector = '.survey-questions';
   const fieldSelector = '.survey-question';
-  const questionTypeSelector = '[name="survey[questions][][question_type]"]';
+  const questionTypeSelector = 'select[name$=\\[question_type\\]]';
   const answerOptionsWrapperSelector = '.survey-question-answer-options';
 
   const autoLabelByPosition = new AutoLabelByPositionComponent({
     listSelector: '.survey-question:not(.hidden)',
     labelSelector: '.card-title span:first',
     onPositionComputed: (el, idx) => {
-      $(el).find('input[name="survey[questions][][position]"]').val(idx);
+      $(el).find('input[name$=\\[position\\]]').val(idx);
     }
   });
 
@@ -35,8 +35,7 @@
 
   const createDynamicFieldsForAnswerOptions = (fieldId) => {
     createDynamicFields({
-      templateId: `survey-question-answer-option-tmpl`,
-      tabsPrefix: `survey-question-answer-option`,
+      placeholderId: `survey-question-answer-option-id`,
       wrapperSelector: `#${fieldId} ${answerOptionsWrapperSelector}`,
       containerSelector: `.survey-question-answer-options-list`,
       fieldSelector: `.survey-question-answer-option`,
@@ -57,8 +56,7 @@
   };
 
   createDynamicFields({
-    templateId: 'survey-question-tmpl',
-    tabsPrefix: 'survey-question',
+    placeholderId: 'survey-question-id',
     wrapperSelector: wrapperSelector,
     containerSelector: '.survey-questions-list',
     fieldSelector: fieldSelector,

decidim-surveys/app/commands/decidim/surveys/admin/update_survey.rb

@@ -38,7 +38,7 @@ def update_survey_questions
               position: form_question.position,
               mandatory: form_question.mandatory,
               question_type: form_question.question_type,
-              answer_options: form_question.answer_options.map { |answer| { "body" => answer.body } }
+              answer_options: form_question.options.map { |answer| { "body" => answer.body } }
             }
 
             if form_question.id.present?

decidim-surveys/app/controllers/decidim/surveys/admin/surveys_controller.rb

@@ -41,7 +41,7 @@ def blank_question
         end
 
         def blank_answer_option
-          @blank_answer_option ||= OpenStruct.new(body: {})
+          @blank_answer_option ||= Admin::SurveyQuestionAnswerOptionForm.new
         end
 
         def question_types

decidim-surveys/app/forms/decidim/surveys/admin/survey_question_answer_option_form.rb

@@ -7,8 +7,11 @@ module Admin
       class SurveyQuestionAnswerOptionForm < Decidim::Form
         include TranslatableAttributes
 
-        attribute :body, String
         translatable_attribute :body, String
+
+        def to_param
+          id || "survey-question-answer-option-id"
+        end
       end
     end
   end

decidim-surveys/app/forms/decidim/surveys/admin/survey_question_form.rb

@@ -7,18 +7,33 @@ module Admin
       class SurveyQuestionForm < Decidim::Form
         include TranslatableAttributes
 
-        attribute :id, String
         attribute :position, Integer
         attribute :mandatory, Boolean, default: false
         attribute :question_type, String
-        attribute :answer_options, Array[SurveyQuestionAnswerOptionForm]
+        attribute :options, Array[SurveyQuestionAnswerOptionForm]
         attribute :deleted, Boolean, default: false
 
         translatable_attribute :body, String
 
         validates :position, numericality: { greater_than_or_equal_to: 0 }
         validates :question_type, inclusion: { in: SurveyQuestion::TYPES }
         validates :body, translatable_presence: true, unless: :deleted
+
+        def map_model(model)
+          self.options = model.answer_options.each_with_index.map do |option, id|
+            [id + 1, option]
+          end
+        end
+
+        def options=(value)
+          @options = value.map do |id, option|
+            SurveyQuestionAnswerOptionForm.new(option.merge(id: id.to_s.to_i))
+          end
+        end
+
+        def to_param
+          id || "survey-question-id"
+        end
       end
     end
   end

decidim-surveys/app/helpers/decidim/surveys/admin/application_helper.rb

@@ -7,15 +7,11 @@ module Admin
       #
       module ApplicationHelper
         def tabs_id_for_question(question)
-          id = question.persisted? ? question.id : "id"
-
-          "survey-question-#{id}"
+          "survey_question_#{question.to_param}"
         end
 
-        def tabs_id_for_question_answer_option(answer_option, idx)
-          id = answer_option.persisted? ? "#{answer_option.question.id}-#{idx}" : "id"
-
-          "survey-question-answer-option-#{id}"
+        def tabs_id_for_question_answer_option(question, answer_option)
+          "survey_question_#{question.to_param}_answer_option_#{answer_option.to_param}"
         end
       end
     end

decidim-surveys/app/models/decidim/surveys/survey_question.rb

@@ -9,12 +9,6 @@ class SurveyQuestion < Surveys::ApplicationRecord
       belongs_to :survey, class_name: "Survey", foreign_key: "decidim_survey_id"
 
       validates :question_type, inclusion: { in: TYPES }
-
-      # Rectify can't handle a hash when using the from_model method so
-      # the answer options must be converted to struct.
-      def answer_options
-        self[:answer_options].map { |option| OpenStruct.new(option) }
-      end
     end
   end
 end

decidim-surveys/app/views/decidim/surveys/admin/surveys/_answer_option.html.erb

@@ -13,12 +13,10 @@
   <div class="card-section">
     <div class="row column">
       <%=
-        translated_field_tag(
-          :text_field_tag,
-          "survey[questions][][answer_options][]",
-          "body",
-          answer_option.body.with_indifferent_access,
-          tabs_id: tabs_id_for_question_answer_option(answer_option, idx),
+        form.translated(
+          :text_field,
+          :body,
+          tabs_id: tabs_id_for_question_answer_option(question, answer_option),
           label: t(".statement"),
           disabled: !survey.questions_editable?,
           enable_tabs: answer_option.persisted?

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

@@ -22,12 +22,10 @@
 
 <div class="survey-questions">
   <% if survey.questions_editable? %>
-    <template id="survey-question-tmpl">
-      <%= render "question", question: blank_question, id: tabs_id_for_question(blank_question) %>
-    </template>
-
-    <template id="survey-question-answer-option-tmpl">
-      <%= render "answer_option", answer_option: blank_answer_option, idx: nil %>
+    <template>
+      <%= fields_for "survey[questions][#{blank_question.to_param}]", blank_question do |question_form| %>
+        <%= render "question", question: blank_question, form: question_form, id: tabs_id_for_question(blank_question) %>
+      <% end %>
     </template>
   <% else %>
     <div class="callout warning">
@@ -37,7 +35,9 @@
 
   <div class="survey-questions-list">
     <% @form.questions.each do |question| %>
-      <%= render "question", question: question, id: tabs_id_for_question(question) %>
+      <%= fields_for "survey[questions][]", question do |question_form| %>
+        <%= render "question", question: question, form: question_form, id: tabs_id_for_question(question) %>
+      <% end %>
     <% end %>
   </div>
 

decidim-surveys/app/views/decidim/surveys/admin/surveys/_question.html.erb

@@ -28,12 +28,9 @@
   <div class="card-section">
     <div class="row column">
       <%=
-        translated_field_tag(
-          :text_field_tag,
-          "survey[questions][]",
-          "body",
-          question.body.with_indifferent_access,
-          object: question,
+        form.translated(
+          :text_field,
+          :body,
           tabs_id: id,
           label: t(".statement"),
           disabled: !survey.questions_editable?,
@@ -44,33 +41,43 @@
 
     <div class="row column">
       <%=
-        check_box_tag(
-          "survey[questions][][mandatory]",
-          "1",
-          question.mandatory,
-          id: "#{id}-mandatory",
-          disabled: !survey.questions_editable?
+        form.check_box(
+          :mandatory,
+          disabled: !survey.questions_editable?,
+          label: t("activemodel.attributes.survey_question.mandatory")
         )
       %>
-      <%= label_tag "", t("activemodel.attributes.survey_question.mandatory"), for: "#{id}-mandatory" %>
     </div>
 
     <div class="row column">
-      <%= label_tag "", t("activemodel.attributes.survey_question.question_type"), for: "#{id}-question_type" %>
-      <%= select_tag "survey[questions][][question_type]", options_from_collection_for_select(question_types, :first, :last, question.question_type), id: "#{id}-question_type", disabled: !survey.questions_editable? %>
+      <%=
+        form.select(
+          :question_type,
+          options_from_collection_for_select(question_types, :first, :last, question.question_type),
+          disabled: !survey.questions_editable?
+        )
+      %>
     </div>
 
     <% if question.persisted? %>
-      <%= hidden_field_tag "survey[questions][][id]", question.id, disabled: !survey.questions_editable? %>
+      <%= form.hidden_field :id, disabled: !survey.questions_editable? %>
     <% end %>
 
-    <%= hidden_field_tag "survey[questions][][position]", question.position || 0, disabled: !survey.questions_editable? %>
-    <%= hidden_field_tag "survey[questions][][deleted]", false, disabled: !survey.questions_editable? %>
+    <%= form.hidden_field :position, value: question.position || 0, disabled: !survey.questions_editable? %>
+    <%= form.hidden_field :deleted, value: false, disabled: !survey.questions_editable? %>
 
     <div class="survey-question-answer-options">
+      <template>
+        <%= fields_for "survey[questions][#{question.to_param}][options][]", blank_answer_option do |answer_option_form| %>
+          <%= render "answer_option", answer_option: blank_answer_option, form: answer_option_form, question: question %>
+        <% end %>
+      </template>
+
       <div class="survey-question-answer-options-list">
-        <% question.answer_options.each_with_index do |answer_option, idx| %>
-          <%= render "answer_option", answer_option: answer_option, idx: idx %>
+        <% question.options.each do |answer_option| %>
+          <%= fields_for "survey[questions][#{question.to_param}][options][]", answer_option do |answer_option_form| %>
+            <%= render "answer_option", answer_option: answer_option, form: answer_option_form, question: question %>
+          <% end %>
         <% end %>
       </div>