Backend Validation for Questions

[crphang]

Currently when adding new questions, a sample data flow works like this.

In InstructorFeedbackQuestionAddAction

FeedbackQuestionAttributes feedbackQuestion = extractFeedbackQuestionData(instructorDetailForCourse.email);
List<String> questionDetailsErrors = feedbackQuestion.getQuestionDetails().validateQuestionDetails();

In extractFeedbackQuestionData:

1. It calls extractQuestionDetails, which already does a validation of the fields, and add to the instance variable.
2. It then serialises those instance variables and store it as Text/Json.

From feedbackQuestion.getQuestionDetails().validateQuestionDetails();:

It picks up the fields that in the metadata, and validates them, returning errors to the user when appropriate. There are a few issues with this flow.

When extractQuestionDetails realises that the fields are invalid, it can do a few things:

1. Put a dummy value to be stored in metaData that signals that an error occured, or leave it null. Then in validateQuestionDetails, we pick up this dummy value and know its an error and tell the user.
2. Simply store the invalid value and allow the validation to be done in validateQuestionDetails and let the user know of the errors.

Both techniques are not perfect due to the coupling between store persistence, extractQuestionDetails and validateQuestionDetails. An implementation that could work is to allow extractQuestionDetails to return the errors since it's already doing the validation, after all, invalid fields should not be permeated in the database.

[shaharyar-shamshi]

@crphang sir As I am able to understand that
FeedbackQuestionAttributes feedbackQuestion = extractFeedbackQuestionData(instructorDetailForCourse.email); first assign the value to the newQuestion then validate it, we have to just validate it before assigning

[crphang]

Based on current design:

1. It assigns a false value, and validate by returning the error message to the user.
2. Or it does not assign any value if field is incorrect, but no appropriate error messages can be shown to the user.

What we can do is let extractQuestionDetails/extractFeedbackQuestionData return the error messages and do validation at the same time. This involves refactoring the questions as well as the controller.

[shaharyar-shamshi]

@crphang sir for each type of question there is their different individual error message which is generated after validating from List<String> questionDetailsErrors = feedbackQuestion.getQuestionDetails().validateQuestionDetails();

[crphang]

@shaharyar-shamshi yes. But validateQuestionDetails, if you inspect its implementation, validateQuestionDetails does validation by checking the meta data, which means the data is already serialized. But problem is, data that is invalid should not be placed in the database in the first place.

And yes, refactoring involves changing all the validateQuestionDetails for all question types.

[shaharyar-shamshi]

@crphang sir should we also throw null parameter error to the user when particular field(if required) is empty empty.

[crphang]

@shaharyar-shamshi, this is left to each question's design of validation. I believe we will add that into the error list to be displayed to user. To clarify further, this issue is more about refactoring the flow of how validation is done, than what the validation is and should be specifically for each question.

[xpdavid]

In V7, there is no extractQuestionDetails. *QuestionDetails will be deserialized from JSON (same effect as extractQuestionDetails but more systematically) and validateQuestionDetails will be called. In other word, the deserialization does not do validation, only validateQuestionDetails will do so.