Skip to content

Commit

Permalink
[5924] Distribute points question reverts to 100
Browse files Browse the repository at this point in the history
* add tests and element name, id changes

* add input validation and processing

* fix checkstyle errors

* run godmode on all tests

* fix NPE in tests

* add TODO, change assertion message
  • Loading branch information
chowyb committed Jul 27, 2016
1 parent 757e5c1 commit f7ed05a
Show file tree
Hide file tree
Showing 48 changed files with 202 additions and 96 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@ public boolean extractQuestionDetails(
String distributeToRecipientsString = null;
String pointsPerOptionString = null;
String pointsString = null;
String pointsForEachOptionString = null;
String pointsForEachRecipientString = null;
String forceUnevenDistributionString = null;
boolean distributeToRecipients = false;
boolean pointsPerOption = false;
Expand All @@ -78,14 +80,28 @@ public boolean extractQuestionDetails(
pointsString =
HttpRequestHelper.getValueFromParamMap(requestParameters,
Const.ParamsNames.FEEDBACK_QUESTION_CONSTSUMPOINTS);
Assumption.assertNotNull("Null points", pointsString);
pointsForEachOptionString =
HttpRequestHelper.getValueFromParamMap(requestParameters,
Const.ParamsNames.FEEDBACK_QUESTION_CONSTSUMPOINTSFOREACHOPTION);
pointsForEachRecipientString =
HttpRequestHelper.getValueFromParamMap(requestParameters,
Const.ParamsNames.FEEDBACK_QUESTION_CONSTSUMPOINTSFOREACHRECIPIENT);

Assumption.assertNotNull("Null points in total", pointsString);
Assumption.assertNotNull("Null points for each option", pointsForEachOptionString);
Assumption.assertNotNull("Null points for each recipient", pointsForEachRecipientString);
forceUnevenDistributionString =
HttpRequestHelper.getValueFromParamMap(requestParameters,
Const.ParamsNames.FEEDBACK_QUESTION_CONSTSUMDISTRIBUTEUNEVENLY);

distributeToRecipients = "true".equals(distributeToRecipientsString);
pointsPerOption = "true".equals(pointsPerOptionString);
points = Integer.parseInt(pointsString);
if (pointsPerOption) {
points = distributeToRecipients ? Integer.parseInt(pointsForEachRecipientString)
: Integer.parseInt(pointsForEachOptionString);
} else {
points = Integer.parseInt(pointsString);
}
forceUnevenDistribution = "on".equals(forceUnevenDistributionString);

if (distributeToRecipients) {
Expand Down Expand Up @@ -198,6 +214,9 @@ public String getQuestionWithExistingResponseSubmissionFormHtml(
Slots.CONSTSUM_POINTS_PER_OPTION, Const.ParamsNames.FEEDBACK_QUESTION_CONSTSUMPOINTSPEROPTION,
Slots.CONSTSUM_NUM_OPTION, Const.ParamsNames.FEEDBACK_QUESTION_CONSTSUMNUMOPTION,
Slots.CONSTSUM_PARAM_POINTS, Const.ParamsNames.FEEDBACK_QUESTION_CONSTSUMPOINTS,
Slots.CONSTSUM_PARAM_POINTSFOREACHOPTION, Const.ParamsNames.FEEDBACK_QUESTION_CONSTSUMPOINTSFOREACHOPTION,
Slots.CONSTSUM_PARAM_POINTSFOREACHRECIPIENT,
Const.ParamsNames.FEEDBACK_QUESTION_CONSTSUMPOINTSFOREACHRECIPIENT,
Slots.CONSTSUM_PARAM_DISTRIBUTE_UNEVENLY, Const.ParamsNames.FEEDBACK_QUESTION_CONSTSUMDISTRIBUTEUNEVENLY
);
}
Expand Down Expand Up @@ -252,6 +271,9 @@ public String getQuestionWithoutExistingResponseSubmissionFormHtml(
Slots.CONSTSUM_POINTS_PER_OPTION, Const.ParamsNames.FEEDBACK_QUESTION_CONSTSUMPOINTSPEROPTION,
Slots.CONSTSUM_NUM_OPTION, Const.ParamsNames.FEEDBACK_QUESTION_CONSTSUMNUMOPTION,
Slots.CONSTSUM_PARAM_POINTS, Const.ParamsNames.FEEDBACK_QUESTION_CONSTSUMPOINTS,
Slots.CONSTSUM_PARAM_POINTSFOREACHOPTION, Const.ParamsNames.FEEDBACK_QUESTION_CONSTSUMPOINTSFOREACHOPTION,
Slots.CONSTSUM_PARAM_POINTSFOREACHRECIPIENT,
Const.ParamsNames.FEEDBACK_QUESTION_CONSTSUMPOINTSFOREACHRECIPIENT,
Slots.CONSTSUM_PARAM_DISTRIBUTE_UNEVENLY, Const.ParamsNames.FEEDBACK_QUESTION_CONSTSUMDISTRIBUTEUNEVENLY
);
}
Expand Down Expand Up @@ -289,6 +311,9 @@ public String getQuestionSpecificEditFormHtml(int questionNumber) {
Slots.CONSTSUM_TO_RECIPIENTS, Const.ParamsNames.FEEDBACK_QUESTION_CONSTSUMTORECIPIENTS,
Slots.CONSTSUM_POINTS_PER_OPTION, Const.ParamsNames.FEEDBACK_QUESTION_CONSTSUMPOINTSPEROPTION,
Slots.CONSTSUM_PARAM_POINTS, Const.ParamsNames.FEEDBACK_QUESTION_CONSTSUMPOINTS,
Slots.CONSTSUM_PARAM_POINTSFOREACHOPTION, Const.ParamsNames.FEEDBACK_QUESTION_CONSTSUMPOINTSFOREACHOPTION,
Slots.CONSTSUM_PARAM_POINTSFOREACHRECIPIENT,
Const.ParamsNames.FEEDBACK_QUESTION_CONSTSUMPOINTSFOREACHRECIPIENT,
Slots.CONSTSUM_PARAM_DISTRIBUTE_UNEVENLY, Const.ParamsNames.FEEDBACK_QUESTION_CONSTSUMDISTRIBUTEUNEVENLY);

}
Expand Down
3 changes: 3 additions & 0 deletions src/main/java/teammates/common/util/Const.java
Original file line number Diff line number Diff line change
Expand Up @@ -637,8 +637,11 @@ public class ParamsNames {
public static final String FEEDBACK_QUESTION_CONSTSUMOPTION = "constSumOption";
public static final String FEEDBACK_QUESTION_CONSTSUMTORECIPIENTS = "constSumToRecipients";
public static final String FEEDBACK_QUESTION_CONSTSUMNUMOPTION = "constSumNumOption";
// TODO: rename FEEDBACK_QUESTION_CONSTSUMPOINTSPEROPTION to a more accurate name
public static final String FEEDBACK_QUESTION_CONSTSUMPOINTSPEROPTION = "constSumPointsPerOption";
public static final String FEEDBACK_QUESTION_CONSTSUMPOINTS = "constSumPoints";
public static final String FEEDBACK_QUESTION_CONSTSUMPOINTSFOREACHOPTION = "constSumPointsForEachOption";
public static final String FEEDBACK_QUESTION_CONSTSUMPOINTSFOREACHRECIPIENT = "constSumPointsForEachRecipient";
public static final String FEEDBACK_QUESTION_CONSTSUMDISTRIBUTEUNEVENLY = "constSumUnevenDistribution";
public static final String FEEDBACK_QUESTION_CONTRIBISNOTSUREALLOWED = "isNotSureAllowedCheck";
public static final String FEEDBACK_QUESTION_GENERATEDOPTIONS = "generatedOptions";
Expand Down
4 changes: 4 additions & 0 deletions src/main/java/teammates/common/util/Templates.java
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,10 @@ public static class Slots {
public static final String CONSTSUM_OPTION_POINT = "${constSumOptionPoint}";
public static final String CONSTSUM_OPTION_VALUE = "${constSumOptionValue}";
public static final String CONSTSUM_PARAM_POINTS = "${Const.ParamsNames.FEEDBACK_QUESTION_CONSTSUMPOINTS}";
public static final String CONSTSUM_PARAM_POINTSFOREACHOPTION =
"${Const.ParamsNames.FEEDBACK_QUESTION_CONSTSUMPOINTSFOREACHOPTION}";
public static final String CONSTSUM_PARAM_POINTSFOREACHRECIPIENT =
"${Const.ParamsNames.FEEDBACK_QUESTION_CONSTSUMPOINTSFOREACHRECIPIENT}";
public static final String CONSTSUM_TO_RECIPIENTS_VALUE = "${constSumToRecipientsValue}";
public static final String CONSTSUM_TO_RECIPIENTS =
"${Const.ParamsNames.FEEDBACK_QUESTION_CONSTSUMTORECIPIENTS}";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,8 @@
distribute among 3 options will be 300 (i.e. 100 x 3)">
<div class="col-xs-4 padding-0">
<input type="number" disabled class="form-control width-100-pc pointsBox"
name="${Const.ParamsNames.FEEDBACK_QUESTION_CONSTSUMPOINTS}"
id="constSumPoints-${questionNumber}" value="${constSumPoints}" min="1"
name="${Const.ParamsNames.FEEDBACK_QUESTION_CONSTSUMPOINTSFOREACHOPTION}"
id="constSumPointsForEachOption-${questionNumber}" value="${constSumPoints}" min="1"
step="1" onChange="updateConstSumPointsValue(${questionNumber})">
</div>
<div class="col-xs-6 padding-0">
Expand All @@ -89,8 +89,8 @@
<div class="col-xs-4 padding-0">
<input type="number" disabled
class="form-control width-100-pc pointsBox"
name="${Const.ParamsNames.FEEDBACK_QUESTION_CONSTSUMPOINTS}"
id="constSumPoints-${questionNumber}" value="${constSumPoints}" min="1"
name="${Const.ParamsNames.FEEDBACK_QUESTION_CONSTSUMPOINTSFOREACHRECIPIENT}"
id="constSumPointsForEachRecipient-${questionNumber}" value="${constSumPoints}" min="1"
step="1" onChange="updateConstSumPointsValue(${questionNumber})">
</div>
<div class="col-xs-6 padding-0">
Expand Down
2 changes: 2 additions & 0 deletions src/main/webapp/js/common.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ var FEEDBACK_QUESTION_CONSTSUMOPTION = 'constSumOption';
var FEEDBACK_QUESTION_CONSTSUMOPTIONTABLE = 'constSumOptionTable';
var FEEDBACK_QUESTION_CONSTSUMTORECIPIENTS = 'constSumToRecipients';
var FEEDBACK_QUESTION_CONSTSUMPOINTS = 'constSumPoints';
var FEEDBACK_QUESTION_CONSTSUMPOINTSFOREACHOPTION = 'constSumPointsForEachOption';
var FEEDBACK_QUESTION_CONSTSUMPOINTSFOREACHRECIPIENT = 'constSumPointsForEachRecipient';
var FEEDBACK_QUESTION_NUMBEROFCHOICECREATED = 'noofchoicecreated';
var FEEDBACK_QUESTION_NUMSCALE_MIN = 'numscalemin';
var FEEDBACK_QUESTION_NUMSCALE_MAX = 'numscalemax';
Expand Down
6 changes: 6 additions & 0 deletions src/main/webapp/js/instructorFeedbackEdit/questionConstSum.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,12 @@ function updateConstSumPointsValue(questionNum) {
if ($('#' + FEEDBACK_QUESTION_CONSTSUMPOINTS + '-' + questionNum).val() < 1) {
$('#' + FEEDBACK_QUESTION_CONSTSUMPOINTS + '-' + questionNum).val(1);
}
if ($('#' + FEEDBACK_QUESTION_CONSTSUMPOINTSFOREACHOPTION + '-' + questionNum).val() < 1) {
$('#' + FEEDBACK_QUESTION_CONSTSUMPOINTSFOREACHOPTION + '-' + questionNum).val(1);
}
if ($('#' + FEEDBACK_QUESTION_CONSTSUMPOINTSFOREACHRECIPIENT + '-' + questionNum).val() < 1) {
$('#' + FEEDBACK_QUESTION_CONSTSUMPOINTSFOREACHRECIPIENT + '-' + questionNum).val(1);
}
}

function addConstSumOption(questionNum) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -394,7 +394,9 @@ public void testExecuteAndPostProcessConstSumOption() {
Const.ParamsNames.FEEDBACK_QUESTION_TYPE, "CONSTSUM",
Const.ParamsNames.FEEDBACK_QUESTION_TEXT, "Split points among the options.",
Const.ParamsNames.FEEDBACK_QUESTION_DESCRIPTION, "more details",
Const.ParamsNames.FEEDBACK_QUESTION_CONSTSUMPOINTS, "100",
Const.ParamsNames.FEEDBACK_QUESTION_CONSTSUMPOINTS, "30",
Const.ParamsNames.FEEDBACK_QUESTION_CONSTSUMPOINTSFOREACHOPTION, "100",
Const.ParamsNames.FEEDBACK_QUESTION_CONSTSUMPOINTSFOREACHRECIPIENT, "50",
Const.ParamsNames.FEEDBACK_QUESTION_CONSTSUMPOINTSPEROPTION, "true",
Const.ParamsNames.FEEDBACK_QUESTION_NUMBEROFCHOICECREATED, "3",
Const.ParamsNames.FEEDBACK_QUESTION_CONSTSUMOPTION + "-1", "Option 1",
Expand Down Expand Up @@ -449,7 +451,9 @@ public void testExecuteAndPostProcessConstSumRecipient() {
Const.ParamsNames.FEEDBACK_QUESTION_TYPE, "CONSTSUM",
Const.ParamsNames.FEEDBACK_QUESTION_TEXT, "Split points among students.",
Const.ParamsNames.FEEDBACK_QUESTION_DESCRIPTION, "more details",
Const.ParamsNames.FEEDBACK_QUESTION_CONSTSUMPOINTS, "100",
Const.ParamsNames.FEEDBACK_QUESTION_CONSTSUMPOINTS, "30",
Const.ParamsNames.FEEDBACK_QUESTION_CONSTSUMPOINTSFOREACHOPTION, "50",
Const.ParamsNames.FEEDBACK_QUESTION_CONSTSUMPOINTSFOREACHRECIPIENT, "100",
Const.ParamsNames.FEEDBACK_QUESTION_CONSTSUMPOINTSPEROPTION, "true",
Const.ParamsNames.FEEDBACK_QUESTION_CONSTSUMTORECIPIENTS, "true",
Const.ParamsNames.FEEDBACK_QUESTION_NUMBEROFCHOICECREATED, "2", // default value.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -772,6 +772,8 @@ public void testExecuteAndPostProcessConstSumOption() throws Exception {
Const.ParamsNames.FEEDBACK_QUESTION_TEXT, "Split points among the options.(edited)",
Const.ParamsNames.FEEDBACK_QUESTION_DESCRIPTION, "more details",
Const.ParamsNames.FEEDBACK_QUESTION_CONSTSUMPOINTS, "100",
Const.ParamsNames.FEEDBACK_QUESTION_CONSTSUMPOINTSFOREACHOPTION, "50",
Const.ParamsNames.FEEDBACK_QUESTION_CONSTSUMPOINTSFOREACHRECIPIENT, "30",
Const.ParamsNames.FEEDBACK_QUESTION_CONSTSUMPOINTSPEROPTION, "false",
Const.ParamsNames.FEEDBACK_QUESTION_NUMBEROFCHOICECREATED, "3",
Const.ParamsNames.FEEDBACK_QUESTION_CONSTSUMOPTION + "-0", "Grades",
Expand Down Expand Up @@ -809,6 +811,8 @@ public void testExecuteAndPostProcessConstSumOption() throws Exception {
Const.ParamsNames.FEEDBACK_QUESTION_TEXT, "Split points among the options.(edited)",
Const.ParamsNames.FEEDBACK_QUESTION_DESCRIPTION, "more details",
Const.ParamsNames.FEEDBACK_QUESTION_CONSTSUMPOINTS, "1000",
Const.ParamsNames.FEEDBACK_QUESTION_CONSTSUMPOINTSFOREACHOPTION, "300",
Const.ParamsNames.FEEDBACK_QUESTION_CONSTSUMPOINTSFOREACHRECIPIENT, "500",
Const.ParamsNames.FEEDBACK_QUESTION_CONSTSUMPOINTSPEROPTION, "false",
Const.ParamsNames.FEEDBACK_QUESTION_NUMBEROFCHOICECREATED, "3",
Const.ParamsNames.FEEDBACK_QUESTION_CONSTSUMOPTION + "-0", "Grades",
Expand Down Expand Up @@ -866,6 +870,8 @@ public void testExecuteAndPostProcessConstSumRecipient() throws Exception {
Const.ParamsNames.FEEDBACK_QUESTION_TEXT, fqd.getQuestionText() + "(edited)",
Const.ParamsNames.FEEDBACK_QUESTION_DESCRIPTION, "more details",
Const.ParamsNames.FEEDBACK_QUESTION_CONSTSUMPOINTS, Integer.toString(fqd.getPoints()),
Const.ParamsNames.FEEDBACK_QUESTION_CONSTSUMPOINTSFOREACHOPTION, Integer.toString(fqd.getPoints()),
Const.ParamsNames.FEEDBACK_QUESTION_CONSTSUMPOINTSFOREACHRECIPIENT, Integer.toString(fqd.getPoints()),
Const.ParamsNames.FEEDBACK_QUESTION_CONSTSUMPOINTSPEROPTION, String.valueOf(fqd.isPointsPerOption()),
Const.ParamsNames.FEEDBACK_QUESTION_NUMBEROFCHOICECREATED, Integer.toString(fqd.getNumOfConstSumOptions()),
Const.ParamsNames.FEEDBACK_QUESTION_CONSTSUMTORECIPIENTS, String.valueOf(fqd.isDistributeToRecipients()),
Expand Down Expand Up @@ -902,6 +908,8 @@ public void testExecuteAndPostProcessConstSumRecipient() throws Exception {
Const.ParamsNames.FEEDBACK_QUESTION_TEXT, fqd.getQuestionText() + "(edited)",
Const.ParamsNames.FEEDBACK_QUESTION_DESCRIPTION, "more details",
Const.ParamsNames.FEEDBACK_QUESTION_CONSTSUMPOINTS, Integer.toString(fqd.getPoints()),
Const.ParamsNames.FEEDBACK_QUESTION_CONSTSUMPOINTSFOREACHOPTION, Integer.toString(fqd.getPoints()),
Const.ParamsNames.FEEDBACK_QUESTION_CONSTSUMPOINTSFOREACHRECIPIENT, Integer.toString(fqd.getPoints()),
Const.ParamsNames.FEEDBACK_QUESTION_CONSTSUMPOINTSPEROPTION, String.valueOf(fqd.isPointsPerOption()),
Const.ParamsNames.FEEDBACK_QUESTION_NUMBEROFCHOICECREATED, Integer.toString(fqd.getNumOfConstSumOptions()),
Const.ParamsNames.FEEDBACK_QUESTION_CONSTSUMTORECIPIENTS, String.valueOf(fqd.isDistributeToRecipients()),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,13 @@ public void testInputValidation() {

feedbackEditPage.fillNewQuestionBox("ConstSum-option qn");
feedbackEditPage.fillNewQuestionDescription("more details");
feedbackEditPage.fillConstSumPointsBox("", -1);

feedbackEditPage.fillConstSumPointsBox("", -1);
assertEquals("1", feedbackEditPage.getConstSumPointsBox(-1));

feedbackEditPage.fillConstSumPointsForEachOptionBox("", -1);
assertEquals("1", feedbackEditPage.getConstSumPointsForEachOptionBox(-1));

feedbackEditPage.clickAddQuestionButton();

feedbackEditPage.verifyStatus("Too little options for Distribute points (among options) question. "
Expand Down Expand Up @@ -133,10 +136,14 @@ public void testAddQuestionAction() throws Exception {
feedbackEditPage.fillNewQuestionBox("const sum qn");
feedbackEditPage.fillNewQuestionDescription("more details");
feedbackEditPage.selectRecipientsToBeStudents();
feedbackEditPage.fillConstSumPointsBox("30", -1);
assertNull(BackDoor.getFeedbackQuestion(courseId, feedbackSessionName, 1));
feedbackEditPage.clickAddQuestionButton();
feedbackEditPage.verifyStatus(Const.StatusMessages.FEEDBACK_QUESTION_ADDED);
assertNotNull(BackDoor.getFeedbackQuestion(courseId, feedbackSessionName, 1));

assertEquals("30", feedbackEditPage.getConstSumPointsBox(1));
assertEquals("30", feedbackEditPage.getConstSumPointsForEachOptionBox(1));
feedbackEditPage.verifyHtmlMainContent("/instructorFeedbackConstSumOptionQuestionAddSuccess.html");
}

Expand All @@ -147,11 +154,14 @@ public void testEditQuestionAction() throws Exception {
feedbackEditPage.clickEditQuestionButton(1);
feedbackEditPage.fillEditQuestionBox("edited const sum qn text", 1);
feedbackEditPage.fillEditQuestionDescription("more details", 1);
feedbackEditPage.fillConstSumPointsBox("200", 1);
feedbackEditPage.selectConstSumPointsOptions("PerOption", 1);
feedbackEditPage.fillConstSumPointsForEachOptionBox("200", 1);

feedbackEditPage.clickSaveExistingQuestionButton(1);
feedbackEditPage.verifyStatus(Const.StatusMessages.FEEDBACK_QUESTION_EDITED);

assertEquals("200", feedbackEditPage.getConstSumPointsBox(1));
assertEquals("200", feedbackEditPage.getConstSumPointsForEachOptionBox(1));

feedbackEditPage.verifyHtmlMainContent("/instructorFeedbackConstSumOptionQuestionEditSuccess.html");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,13 @@ public void testInputValidation() {

feedbackEditPage.fillNewQuestionBox("ConstSum-recipient qn");
feedbackEditPage.fillNewQuestionDescription("more details");
feedbackEditPage.fillConstSumPointsBox("", -1);

feedbackEditPage.fillConstSumPointsBox("", -1);
assertEquals("1", feedbackEditPage.getConstSumPointsBox(-1));

feedbackEditPage.fillConstSumPointsForEachRecipientBox("", -1);
assertEquals("1", feedbackEditPage.getConstSumPointsForEachRecipientBox(-1));

assertFalse(feedbackEditPage.isElementVisible("constSumOptionTable--1"));

feedbackEditPage.clickDiscardChangesLink(-1);
Expand All @@ -87,7 +91,7 @@ public void testCustomizeOptions() {
______TS("CONST SUM: set points options");

feedbackEditPage.selectConstSumPointsOptions("PerRecipient", -1);
feedbackEditPage.fillConstSumPointsBox("100", -1);
feedbackEditPage.fillConstSumPointsForEachRecipientBox("30", -1);

}

Expand All @@ -102,6 +106,9 @@ public void testAddQuestionAction() throws Exception {
feedbackEditPage.clickAddQuestionButton();
feedbackEditPage.verifyStatus(Const.StatusMessages.FEEDBACK_QUESTION_ADDED);
assertNotNull(BackDoor.getFeedbackQuestion(courseId, feedbackSessionName, 1));

assertEquals("30", feedbackEditPage.getConstSumPointsBox(1));
assertEquals("30", feedbackEditPage.getConstSumPointsForEachRecipientBox(1));
feedbackEditPage.verifyHtmlMainContent("/instructorFeedbackConstSumRecipientQuestionAddSuccess.html");
}

Expand All @@ -117,6 +124,9 @@ public void testEditQuestionAction() throws Exception {

feedbackEditPage.clickSaveExistingQuestionButton(1);
feedbackEditPage.verifyStatus(Const.StatusMessages.FEEDBACK_QUESTION_EDITED);

assertEquals("200", feedbackEditPage.getConstSumPointsBox(1));
assertEquals("200", feedbackEditPage.getConstSumPointsForEachRecipientBox(1));

feedbackEditPage.verifyHtmlMainContent("/instructorFeedbackConstSumRecipientQuestionEditSuccess.html");
}
Expand Down

0 comments on commit f7ed05a

Please sign in to comment.