Questionnaire answer order wrong and others [#2209443]

The answers in the questionnaire where wrong if the answers where reordered in the salsa backend.
Also the default value for fields with multiple values was not noted.
I also put the fields each in one container and added a little more documentation to understand how the salsa options where parsed etc.

Comment	File	Size	Author
#4	salsa_entity_questionnaire_elements-2209443-4.interdiff.txt	2.09 KB	s_leu
#4	salsa_entity_questionnaire_elements-2209443-4.patch	34.47 KB	s_leu
#1	salsa_entity_questionnaire_elements_1.interdiff.txt	848 bytes	LukyLuke_ch
#1	salsa_entity_questionnaire_elements_2.patch	4.39 KB	LukyLuke_ch
	salsa_entity_questionnaire_elements_1.patch	4.36 KB	LukyLuke_ch

Comments

Comment #1

LukyLuke_ch commented 4 March 2014 at 12:07

Status	File	Size
new	salsa_entity_questionnaire_elements_2.patch	4.39 KB
new	salsa_entity_questionnaire_elements_1.interdiff.txt	848 bytes

Missed that the question key in the container must be the same as before and not the container.

Comment #2

s_leu commented 3 April 2014 at 08:10

Status:

Needs review

» Needs work

+++ b/salsa_questionnaire/salsa_questionnaire.module
@@ -211,69 +211,93 @@ function salsa_questionnaire_form_submit($form, &$form_state) {
+      $element += array(
+        '#default_value' => ($question->Response_Type == 'Multiple Choice (Checkbox)') ? explode(',', $response->Response) : $response->Response,
+      );
...
+    $element += array(
+      '#default_value' => $default[1],
+    );
...
+      $element += array(
+        '#type' => 'textfield',
+      );
...
+      $element += array(
         '#type' => 'textarea',
       );

For single additions to the array I suggest using $element['#default_value'] rather than += array()

+++ b/salsa_questionnaire/salsa_questionnaire.module
@@ -211,69 +211,93 @@ function salsa_questionnaire_form_submit($form, &$form_state) {
+    $default = explode('~', $question->Default_Value);
+    $element += array(
+      '#default_value' => $default[1],

After applying the patch and visiting a site with a questionnaire, this code threw notices. Guess the split by ~ should happen outside of the condition. The condition should also check if default[1] contains anything.

Comment #3

LukyLuke_ch commented 3 April 2014 at 08:42

Status:

Needs work

» Needs review

For single additions to the array I suggest using $element['#default_value'] rather than += array()

The operator "+=" on arrays adds the new values only if they not already exists - the "$a[]" adds them and overrides existing what we not want here. This is a short version of "$element = array_merge(array('foo' => 'bar'), $element)".

After applying the patch and visiting a site with a questionnaire, this code threw notices. Guess the split by ~ should happen outside of the condition. The condition should also check if default[1] contains anything.

What notices do you get here and on which line? On the explode, the condition or by accessing on the key 1?
In general: We first need to check if there is a "Default_Value", after we can split this up in the various parts, no?
If the "Default_Value" is provided the right way from salsa (and the questionnaire is configured correctly), then there should be no problem with this. The first index holds the field_label and the second the field_name. So if the user has not entered a label for the option, there should also be an empty string in the frontend - also for the default value.

Comment #4

s_leu commented 18 June 2014 at 08:57

Status	File	Size
new	salsa_entity_questionnaire_elements-2209443-4.patch	34.47 KB
new	salsa_entity_questionnaire_elements-2209443-4.interdiff.txt	2.09 KB

What notices do you get here and on which line?

The problem is that you just used the same default value without considering the element type. Elements of type "checkboxes" expect an array instead of a string. Adding a new patch that considers this.