S2U-12 5.1.1.8 Tests & Quizzes: Improving i18n and feedback on calcul…

…ated questions - MASTER (sakaiproject#11737)

samigo/samigo-api/src/java/org/sakaiproject/tool/assessment/data/ifc/assessment/ItemDataIfc.java

@@ -152,19 +152,23 @@ public interface ItemDataIfc extends Comparable<ItemDataIfc>, java.io.Serializab
 
   String getCorrectItemFeedback();
 
-  void setCorrectItemFeedback(String text);
+  String getCorrectItemFeedbackValue();
+
+  void setCorrectItemFeedback(String text, String value);
 
   String getInCorrectItemFeedback();
 
-  void setInCorrectItemFeedback(String text);
+  String getInCorrectItemFeedbackValue();
+
+  void setInCorrectItemFeedback(String text, String value);
 
   String getGeneralItemFeedback();
 
-  void setGeneralItemFeedback(String text);
+  void setGeneralItemFeedback(String text, String value);
 
   String getItemFeedback(String typeId);
 
-  void addItemFeedback(String typeId, String text);
+  void addItemFeedback(String typeId, String text, String value);
 
   void removeFeedbackByType(String typeId);
 
@@ -224,4 +228,5 @@ public interface ItemDataIfc extends Comparable<ItemDataIfc>, java.io.Serializab
 
   public Long getOriginalItemId();
   public void setOriginalItemId(Long originalItemId);
+  public void updateFeedbackByType(String typeId, String text, String value);
 }

samigo/samigo-api/src/java/org/sakaiproject/tool/assessment/data/ifc/assessment/ItemFeedbackIfc.java

@@ -48,4 +48,8 @@ public interface ItemFeedbackIfc
 
   void setText(String text);
 
+  String getTextValue();
+
+  void setTextValue(String textValue);
+
 }

samigo/samigo-app/src/java/org/sakaiproject/tool/assessment/bundle/AuthorMessages.properties

@@ -178,6 +178,8 @@ samigo_formula_error_5=5: Unexpected Part
 samigo_formula_error_6=6: Unexpected end of Expression 
 samigo_formula_error_7=7: Value expected
 samigo_formula_error_9=9: Variable not defined in instructions
+samigo_formula_error_10=10: Variable not defined in correct answer feedback
+samigo_formula_error_11=11: Variable not defined in incorrect answer feedback
 samigo_formula_error_101=101: Unknown operator
 samigo_formula_error_102=102: Unknown expression
 samigo_formula_error_103=103: Unknown variable
@@ -589,7 +591,6 @@ submissions_allowed_error=The Time Allowed must be a positive integer. Example v
 corrAnswer=You must select at least one Answer.
 partial_credit_limit_summary="% Value" for an incorrect answer selection needs to be a whole number between 0 and 99.
 partial_credit_limit_detail=Value needs to be a whole number between 0 and 99.
-feedbackTooLong=The provided feedback text is too long. Correct, incorrect, general, and selection level feedback cannot exceed {0} characters each.
 
 edit_published_assessment_warn_1=This assessment has been retracted from student view.  
 edit_published_assessment_warn_21=Click "Republish" or "Regrade and Republish" to make it available to students.

samigo/samigo-app/src/java/org/sakaiproject/tool/assessment/bundle/AuthorMessages_ca.properties

@@ -178,6 +178,8 @@ samigo_formula_error_5=5\: Part no esperada
 samigo_formula_error_6=6\: L\u2019expressi\u00f3 acaba de manera inesperada
 samigo_formula_error_7=7\: Es requereix un valor
 samigo_formula_error_9=9\: La variable no est\u00e0 definida a les instruccions
+samigo_formula_error_10=10: Aquesta variable no est\u00e0 definida en el comentari a la resposta correcta
+samigo_formula_error_11=11: Aquesta variable no est\u00e0 definida en el comentari a la resposta incorrecta
 samigo_formula_error_101=101\: Operador desconegut
 samigo_formula_error_102=102\: Expressi\u00f3 desconeguda
 samigo_formula_error_103=103\: Variable desconeguda

samigo/samigo-app/src/java/org/sakaiproject/tool/assessment/bundle/AuthorMessages_es.properties

@@ -178,6 +178,8 @@ samigo_formula_error_5=5\: Parte inesperada
 samigo_formula_error_6=6\: Final inesperado de la expresi\u00f3n 
 samigo_formula_error_7=7\: Valor esperado
 samigo_formula_error_9=9\: Variable no definida en las instrucciones
+samigo_formula_error_10=10: Variable no definida en el comentario a la respuesta correcta
+samigo_formula_error_11=11: Variable no definida en el comentario a la respuesta incorrecta
 samigo_formula_error_101=101\: Operador desconocido
 samigo_formula_error_102=102\: Expresi\u00f3n desconocida
 samigo_formula_error_103=103\: Variable desconocida
@@ -585,7 +587,6 @@ submissions_allowed_error=El tiempo permitido debe ser un n\u00famero entero pos
 corrAnswer=Debe seleccionar al menos una respuesta.
 partial_credit_limit_summary=El "% del Valor" para la selecci\u00f3n de una respuesta incorrecta debe ser un n\u00famero entero entre 0 y 99.
 partial_credit_limit_detail=El valor debe ser un n\u00famero entero entre 0 y 99.
-feedbackTooLong=El texto de feedback introducido es demasiado largo. Los comentarios a respuesta correcta, incorrecta, general y de nivel de selecci\u00f3n no pueden ser mayores de {0} caracteres.
 
 edit_published_assessment_warn_1=Este examen se ha retirado temporalmente de la vista del alumnado.
 edit_published_assessment_warn_21=Haga clic en "Republicar" o en "Republicar y recalificar" para que el examen est\u00e9 disponible al alumnado.

samigo/samigo-app/src/java/org/sakaiproject/tool/assessment/bundle/AuthorMessages_eu.properties

@@ -178,6 +178,8 @@ samigo_formula_error_5=5\: Espero ez den partea
 samigo_formula_error_6=6\: Adierazpen batentzat espero ez den bukaera 
 samigo_formula_error_7=7\: Espero den balioa
 samigo_formula_error_9=9\: Aldagaia ez dago definitua jarraibideetan
+samigo_formula_error_10=10: Aldagaia ez dago definituta feedbackean (ongi dagoen erantzuna)
+samigo_formula_error_11=11: Aldagaia ez dago definituta feedbackean (gaizki dagoen erantzuna)
 samigo_formula_error_101=101\: Eragilea ezezaguna da
 samigo_formula_error_102=102\: Adierazpena ezezaguna da
 samigo_formula_error_103=103\: Aldagaia ezezaguna da

samigo/samigo-app/src/java/org/sakaiproject/tool/assessment/ui/bean/author/ItemBean.java

@@ -22,7 +22,6 @@
 package org.sakaiproject.tool.assessment.ui.bean.author;
 
 import java.io.Serializable;
-import java.text.MessageFormat;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
@@ -40,9 +39,9 @@
 
 import lombok.Getter;
 import lombok.Setter;
-import org.apache.commons.lang3.StringUtils;
 
 import org.sakaiproject.tool.assessment.data.ifc.assessment.ItemDataIfc;
+import org.apache.commons.lang3.StringUtils;
 import org.sakaiproject.component.cover.ServerConfigurationService;
 import org.sakaiproject.tool.api.Placement;
 import org.sakaiproject.tool.assessment.facade.TypeFacade;
@@ -52,7 +51,6 @@
 import org.sakaiproject.tool.assessment.data.dao.assessment.FavoriteColChoices;
 import org.sakaiproject.tool.assessment.data.dao.assessment.FavoriteColChoicesItem;
 import org.sakaiproject.tool.cover.ToolManager;
-import static org.sakaiproject.tool.assessment.ui.listener.author.ItemAddListener.MAX_FEEDBACK_CHARS;
 import org.sakaiproject.util.ResourceLoader;
 
 
@@ -1140,13 +1138,6 @@ public boolean isMatchError(){
 	    context.addMessage(null,new FacesMessage(RB_AUTHOR_MESSAGES.getString("match_error")));
 	    return true;
 	}
-
-    // Choice level feedback cannot exceed 4000 characters
-    if(StringUtils.length(currentMatchPair.getCorrMatchFeedback()) > MAX_FEEDBACK_CHARS || StringUtils.length(currentMatchPair.getIncorrMatchFeedback()) > MAX_FEEDBACK_CHARS) {
-        FacesContext context = FacesContext.getCurrentInstance();
-        context.addMessage(null, new FacesMessage(MessageFormat.format(RB_AUTHOR_MESSAGES.getString("feedbackTooLong"), new Object[]{MAX_FEEDBACK_CHARS})));
-        return true;
-    }
 	return false;
     }
 

samigo/samigo-app/src/java/org/sakaiproject/tool/assessment/ui/bean/delivery/ItemContentsBean.java

@@ -86,6 +86,8 @@ public class ItemContentsBean implements Serializable {
 
 	private String feedback;
 
+	private String feedbackValue;
+
 	private String responseId = "2";
 
 	private String responseText = "";
@@ -649,6 +651,25 @@ public void setFeedback(String feedback) {
 		this.feedback = feedback;
 	}
 
+	/**
+	 * item level feedback value
+	 *
+	 * @return the item level feedback value
+	 */
+	public String getFeedbackValue() {
+		return feedbackValue;
+	}
+
+	/**
+	 * item level feedback value
+	 *
+	 * @param feedbackValue
+	 *            the item level feedback value
+	 */
+	public void setFeedbackValue(String feedbackValue) {
+		this.feedbackValue = feedbackValue;
+	}
+
 	/**
 	 * If this is a true-false question return true if it is true, else false.
 	 * If it is not a true-false question return false.

samigo/samigo-app/src/java/org/sakaiproject/tool/assessment/ui/listener/author/CalculatedQuestionExtractListener.java

@@ -28,6 +28,8 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 
 import javax.faces.application.FacesMessage;
 import javax.faces.context.FacesContext;
@@ -49,6 +51,9 @@
 public class CalculatedQuestionExtractListener implements ActionListener{
     private static final String ERROR_MESSAGE_BUNDLE = "org.sakaiproject.tool.assessment.bundle.AuthorMessages";
     private static final int MAX_ATTEMPT_CNT = 100;
+    private static final Pattern CALCQ_ALLOWING_VARIABLES_FORMULAS_NAMES = Pattern.compile("[a-zA-ZÀ-ÿ\\u00f1\\u00d1]\\w*", Pattern.UNICODE_CHARACTER_CLASS);
+    private static final String VARIABLE_ON_CORRECT_FEEDBACK_NOT_IN_INSTRUCTIONS = "10";
+    private static final String VARIABLE_ON_INCORRECT_FEEDBACK_NOT_IN_INSTRUCTIONS = "11";
 
     /**
      * This listener will read in the instructions, parse any variables and 
@@ -105,17 +110,32 @@ public List<String> validate(ItemBean item,boolean extracting) {
 
         GradingService service = new GradingService();
         String instructions = item.getInstruction();
+        String corrFeedback = item.getCorrFeedback();
+        String incorrFeedback = item.getIncorrFeedback();
         List<String> formulaNames = service.extractFormulas(instructions);
         List<String> variableNames = service.extractVariables(instructions);
+        List<String> corrformulaNames = service.extractFormulas(corrFeedback);
+        List<String> corrvariableNames = service.extractVariables(corrFeedback);
+        List<String> incorrformulaNames = service.extractFormulas(incorrFeedback);
+        List<String> incorrvariableNames = service.extractVariables(incorrFeedback);
 
         errors.addAll(validateExtractedNames(variableNames, formulaNames));
+        errors.addAll(validateExtractedNames(corrvariableNames, corrformulaNames));
+        errors.addAll(validateExtractedNames(incorrvariableNames, incorrformulaNames));
+
+        // checking if there are variable names on feedback which are not in instructions.
+        errors.addAll(checkVariableNamesOnFeedback(variableNames, corrvariableNames, VARIABLE_ON_CORRECT_FEEDBACK_NOT_IN_INSTRUCTIONS));
+        errors.addAll(checkVariableNamesOnFeedback(variableNames, incorrvariableNames, VARIABLE_ON_INCORRECT_FEEDBACK_NOT_IN_INSTRUCTIONS));
 
         // add new variables and formulas
         // verify that at least one variable and formula are defined
         if (errors.size() == 0) {
             errors.addAll(createFormulasFromInstructions(item, formulaNames));
             errors.addAll(createVariablesFromInstructions(item, variableNames));
-            errors.addAll(createCalculationsFromInstructions(item.getCalculatedQuestion(), instructions, service));
+            item.getCalculatedQuestion().clearCalculations(); // reset the current set and extract a new one
+            errors.addAll(createCalculationsFromInstructionsOrFeedback(item.getCalculatedQuestion(), instructions, service));
+            errors.addAll(createCalculationsFromInstructionsOrFeedback(item.getCalculatedQuestion(), corrFeedback, service));
+            errors.addAll(createCalculationsFromInstructionsOrFeedback(item.getCalculatedQuestion(), incorrFeedback, service));
         }
 
         // validate variable min and max and formula tolerance
@@ -238,21 +258,20 @@ private List<String> createVariablesFromInstructions(ItemBean item, List<String>
     }
 
     /**
-     * Finds the calculations in the instructions and places them in the CalculatedQuestionBean
-     * (destroys anything that was already there)
+     * Finds the calculations in the instructions or feedback and places them in the CalculatedQuestionBean
      * 
      * @param calculatedQuestionBean
-     * @param instructions
+     * @param instructionsorfeedback
      * @param service
      * @return list of error messages (empty if there are none)
      */
-    static List<String> createCalculationsFromInstructions(CalculatedQuestionBean calculatedQuestionBean, String instructions, GradingService service) {
+    static List<String> createCalculationsFromInstructionsOrFeedback(CalculatedQuestionBean calculatedQuestionBean, String instructionsorfeedback, GradingService service) {
         List<String> errors = new ArrayList<String>();
-        calculatedQuestionBean.clearCalculations(); // reset the current set and extract a new one
-        if (instructions.indexOf(GradingService.CALCULATION_AUX_OPEN) != -1 || instructions.indexOf(GradingService.CALCULATION_AUX_CLOSE) != -1) {
+
+        if (instructionsorfeedback.indexOf(GradingService.CALCULATION_AUX_OPEN) != -1 || instructionsorfeedback.indexOf(GradingService.CALCULATION_AUX_CLOSE) != -1) {
             errors.add(getErrorMessage("calc_question_simple_instructions_step_4"));
         }
-        List<String> calculations = service.extractCalculations(instructions);
+        List<String> calculations = service.extractCalculations(instructionsorfeedback);
         if (!calculations.isEmpty()) {
             for (String calculation : calculations) {
                 CalculatedQuestionCalculationBean calc = new CalculatedQuestionCalculationBean(calculation);
@@ -277,7 +296,7 @@ private List<String> validateExtractedNames(List<String> variableNames, List<Str
             if (formula == null || formula.length() == 0) {
                 errors.add(getErrorMessage("formula_name_empty"));
             } else {
-                if (!formula.matches("[a-zA-Z]\\w*")) {
+                if (!CALCQ_ALLOWING_VARIABLES_FORMULAS_NAMES.matcher(formula).matches()) {
                     errors.add(getErrorMessage("formula_name_invalid"));
                 }
             }
@@ -288,7 +307,7 @@ private List<String> validateExtractedNames(List<String> variableNames, List<Str
             if (variable == null || variable.length() == 0) {
                 errors.add(getErrorMessage("variable_name_empty"));
             } else {
-                if (!variable.matches("[a-zA-Z]\\w*")) {
+                if (!CALCQ_ALLOWING_VARIABLES_FORMULAS_NAMES.matcher(variable).matches()) {
                     errors.add(getErrorMessage("variable_name_invalid"));
                 }
             }
@@ -611,4 +630,24 @@ private static String getErrorMessage(String errorCode) {
         return err;
     }
 
+    /**
+     * checkVariableNamesOnFeedback() retrieves the variables which are on feedback and are missing on instructions
+     * the errorCode
+     * @param variableNames feedback
+     * @return
+     */
+    private static List<String> checkVariableNamesOnFeedback(List<String> variableNames, List<String> feedback, String nerror) {
+        List<String> errors = new ArrayList<String>();
+
+        List<String> result = new ArrayList<>(variableNames);
+        result.addAll(feedback);
+        result.removeAll(variableNames);
+
+        if (!result.isEmpty()){
+            String msg = getErrorMessage("samigo_formula_error_" + nerror);
+            errors.add(msg + " :" + result.toString());
+        }
+        return errors;
+    }
+
 }