Expertiza_Wiki - User contributions [en]

CSC/ECE 517 Spring 2023 -E2326 Refactor questionnaires controller.rb

2023-05-02T22:07:02Z

Jtdavis9: Added to Testing section

__TOC__

= Background =

In Expertiza, Questionnaires are used primarily by instructors to create question forms/ reviews, etc. As a student, you might have seen them while performing reviews for peers. The questionnaire is the superclass for all kinds of questionnaires and rubrics—rubrics for evaluating submissions and teammate contributions and taking quizzes and surveys. All of these are subclasses of the Questionnaire. questionnaires_controller.rb is charged with creating, displaying, and managing Questionnaires. Because it is used so widely, malfunctions could cause bugs in many parts of the system; hence it is especially important that the code be clear. 
A Questionnaire can have multiple Questions as a part of it. These questionnaires can be used as a part of an assignment.
Also, we notice that the Questionnaire controller has a parent-child relationship with the QuizQuestionnaire controller. There are multiple types of questions that can be added to a questionnaire like Criterion, Dropdown, Checkbox, and TextArea to name a few. There are multiple type of Questionnaires like Review, MetaReview, Assignment etc. 
To sum it off, we can have many types of Questionnaires and each would have possibly multiple types of Questions. There can be numerous questionnaires of the same kind for a given Questionnaire type.

= Project Purpose =

In recent years, <code>questionnaires_controller.rb</code> has been refactored repeatedly. It is a lot clearer than it used to be. But rough edges still remain.
question.rb contains a large number of constants. It is not clear what they are used for.
There is a questionnaires_controller, but no questions_controller which is fully developed. The questionnaires_controller creates Question objects. 
This is not elegant, because a controller should only create objects of one type, the type of objects it controls.
In the present code we see that the <code>questionnaire_controller.rb</code> has multiple non CRUD methods that should have been in the <code>questions_controller.rb</code>. Our priority is to refactor these functions to the <code>questions_controller.rb</code> so that we maintain the Single responsibility in the SOLID principles. We also notice that there are 2 places where there is code that is repeating itself so we extract the common code into a helper function to make the code more DRY. 
The following functions that need refactoring-
* <code>def delete_questions(questionnaire_id)</code> Move to questions_controller
* <code>def question_params</code> Move to questions_controller
* <code>def save_new_questions(questionnaire_id)</code> Move to questions_controller
* <code>def save_questions(questionnaire_id)</code> Move to questions_controller

We also noticed that there were many places that used constants like the text file are size or the default min and max scores being hard coded. This makes the code not suitable for extension as any change would need to be refactored in multiple places and break code easily. 
Thus we plan to modify the <code> models/questions.rb</code> and add the constants there so that all files related to questions can have access to these constants. This would make the refactoring process very easy.
 

= Design Pattern Used =

There are multiple types of questions created based on the parameters passed by the user to the controller. This led to two things, Objects being created dynamically which might not be of the acceptable kind, and code injection, as we are using unsanitized user input.

We have decided to use the '''Factory design''' pattern instead where we create a type of Questionnaire based on the type of parameters passed. Hence instead of multiple if/else code blocks to check the kind of input passed, we use a factory method to create and return the Questionnaire/Question object. Additionally, Instead of using if/else checks in the factory method, we opt for using a Map to match the input param to an Object type which further cleaned our code.

We employed '''SOLID''' principles, and shifted code that pertained to Questions from Questionnaire controller to Questions controller. This helps us better adhere to the '''Single Responsibility''' principle. We also shifted the common code out of the Questionnaire controller to helper classes to make the code more '''DRY'''.

= All Changes Implemented =

{| class="wikitable" style="width: 100%;
!  #  !! File/Function !! Change Proposed !! Rationale !! Commit Link
|-
|1
|method <code>add_new_questions()</code>
|Store the used default value to some kind of constant in <code>questions.rb</code>, so that they can be changed without refactoring code
|There are many literals used in the code and can change in future, so it is better to separate them
|[https://github.com/SoulTrekker21/expertiza/commit/4721039e523505e2c07f0691d8a7d42af2dc87ba Commit]
|-
|2
|files<code>question_helper.rb</code> and <code>questionnaire_helper.rb</code>
|Add a factory method that creates respective object based on the parameter that is being passed
|We use factory method in a helper file as that removes the code for creating an object from inside a function that just uses those objects
|[https://github.com/SoulTrekker21/expertiza/commit/4721039e523505e2c07f0691d8a7d42af2dc87ba Commit]
|-
|3
|File <code>questionnaires_controller.rb</code>
|Move <code>save_new_questions()</code>
|To be consistent with SOLID principle, these methods belong in questions_controller.rb
|[https://github.com/SoulTrekker21/expertiza/commit/9d3a846d862cfe4661c858e98902876cfe789a64 Commit]
|-
|4
|File <code>questionnaires_controller.rb</code>
|Move <code>delete_questions()</code>
|To be consistent with SOLID principle, these methods belong in questions_controller.rb
|[https://github.com/SoulTrekker21/expertiza/commit/669fc8e47c7a81223d48748f785af10b359f942e Commit]
|-
|5
|File <code>questionnaires_controller.rb</code>
|Move <code>save_questions()</code>
|To be consistent with SOLID principle, these methods belong in questions_controller.rb
|[https://github.com/SoulTrekker21/expertiza/commit/4edd184930109b920f7cf714b10dd22fd62c1f41 Commit]
|-
|6
|file <code>questionnaires_controller.rb</code> and <code>questions_controller.rb</code>
|Refactor or add more comments to the following vague method names
|There are confusing function names like <code>save_all_questions() </code> , <code>save_new_questions() </code> , <code>save_questions() </code> and <code>save() </code> and the use of each cannot be easily understood
|[https://github.com/SoulTrekker21/expertiza/commit/9edb52d359a1e341985259829bc7b4ddd224663a Commit]
|-
|7
|method <code>save_questions()</code>
|Change <code>params[:question].keys.each</code> to <code>params[:question].each_key</code>
| This will improve the performance and uses a single enumerator
|[https://github.com/Soultrekker21/expertiza/commit/690f8afe526d79c5811dcfbbc5dad826c1fd2c7c Commit]
|-
|8
|method <code>create()</code>
|Remove parenthesis around function call on line 72
|Unnecessary parenthesis
|[https://github.com/SoulTrekker21/expertiza/commit/4721039e523505e2c07f0691d8a7d42af2dc87ba Commit]
|-
|9
|method <code>create()</code> in <code>questionnaire_controller.rb</code>
|Move code related to creation of tree node to different function name <code>create_tree_node()</code>
|The create method is very big and this would help in readability and code re-usability
|[https://github.com/SoulTrekker21/expertiza/commit/4721039e523505e2c07f0691d8a7d42af2dc87ba Commit]
|-
|10
|method <code>questionnaire_helper()</code>
|Remove the usage of if/else or switch case and use a Hash Map for questionnaire factory object creation
|We want to make our code more modular and extendable. Any change can be easily added to Hash Map rather than adding if else statements.
|[https://github.com/SoulTrekker21/expertiza/commit/a24b18cb1702188a4d04483de8ca407920fef775 Commit]
|-
|11
|method <code>question_helper()</code>
|Remove the usage of if/else or switch case and use a Hash Map for questions factory object creation
|We want to make our code more modular and extendable. Any change can be easily added to Hash Map rather than adding if else statements.
|[https://github.com/SoulTrekker21/expertiza/commit/a24b18cb1702188a4d04483de8ca407920fef775 Commit]
|-
|12
|controller <code>advice_controller.rb</code>
|There needs to be a <code>.each</code> after they keys are enumerated
|The Edit/View advice button was not working on the view where we create a questionnaire and new questions
|[https://github.com/SoulTrekker21/expertiza/commit/690f8afe526d79c5811dcfbbc5dad826c1fd2c7c Commit]
|-
|13
|file <code>routes.rb</code>
|Add some routes so that all the new methods in <code>questions_controller.rb</code> can work
|Newly added functions don't have a predefined route and it must be specified
|[https://github.com/SoulTrekker21/expertiza/commit/843a12bce18bf441c8cb7e5b2a04d905bb155418 Commit]
|-
|14
|file <code>routes.rb</code>
|Add some routes so that the delete functionality works for questionnaires
|The existing code had the functionality implemented but the route was not configured rendered an error
|[https://github.com/SoulTrekker21/expertiza/commit/6647464ad7c478ad7cd6ba27beecdee2cfa492f9 Commit]
|-
|15
|controller <code>quiz_questionnaire_controller.rb()</code>
|There are multiple redirects to functions in the <code>questionnaire_controller.rb</code>
|As some functions have been moved to the <code>questions_controller.rb</code> they need to be refactored.
|[https://github.com/SoulTrekker21/expertiza/commit/7b875fbcbc0bc23db2d14280945f09c4cc968223 Commit]
|-
|16
|method <code>update_questionnaire_questions</code>
|As per DRY, we removed the code to update the questionnaire's questions from <code>questionnaire_controller.rb</code> to the helper class.
|This will make the code more DRY and maintainable
|[https://github.com/Soultrekker21/expertiza/commit/b8b504aeeb459958115fa4aed4df2cc1c61ddaf3 Commit]
|}

= Sample of Code Changes =
*Questionnaire Factory where the object is created with switch case using a HashMap in <code>questionnaire_helper.rb</code>
#Map type to questionnaire
QUESTIONNAIRE_MAP = {
'ReviewQuestionnaire' => ReviewQuestionnaire,
'MetareviewQuestionnaire' => MetareviewQuestionnaire,
'AuthorFeedbackQuestionnaire' => AuthorFeedbackQuestionnaire,
'TeammateReviewQuestionnaire' => TeammateReviewQuestionnaire,
'AssignmentSurveyQuestionnaire' => AssignmentSurveyQuestionnaire,
'SurveyQuestionnaire' => SurveyQuestionnaire,
'GlobalSurveyQuestionnaire' => GlobalSurveyQuestionnaire,
'CourseSurveyQuestionnaire' => CourseSurveyQuestionnaire,
'BookmarkRatingQuestionnaire' => BookmarkRatingQuestionnaire,
'QuizQuestionnaire' => QuizQuestionnaire
}.freeze

# factory method to create the appropriate questionnaire based on the type
def questionnaire_factory(type)
questionnaire = QUESTIONNAIRE_MAP[type]
if questionnaire.nil?
flash[:error] = 'Error: Undefined Questionnaire'
else
questionnaire.new
end
end

*Question Factory where the questions object is created using a HashMap in <code>question_helper.rb</code>
module QuestionHelper
# Maps type to question
QUESTION_MAP = {
'Criterion' => Criterion,
'Scale' => Scale,
'Cake' => Cake,
'Dropdown' => Dropdown,
'Checkbox' => Checkbox,
'TextArea' => TextArea,
'TextField' => TextField,
'UploadFile' => UploadFile,
'SectionHeader' => SectionHeader,
'TableHeader' => TableHeader,
'ColumnHeader' => ColumnHeader
}.freeze

# factory method to create the appropriate question based on the type
def question_factory(type, questionnaire_id, seq)
question_class = QUESTION_MAP[type]
if question_class.nil?
flash[:error] = 'Error: Undefined Question'
else
question_class.create(txt: '', questionnaire_id: questionnaire_id, seq: seq, type: type, break_before: true)
end
end
end

*<code>models/question.rb</code> that shows how the constants are used to store the parameters that would be used for object creation
# Class variables - used questionnaires_controller.rb to set the parameters for a question.
MAX_LABEL = 'Strongly agree'.freeze
MIN_LABEL = 'Strongly disagree'.freeze
SIZES = { 'Criterion' => '50, 3', 'Cake' => '50, 3', 'TextArea' => '60, 5', 'TextField' => '30' }.freeze
ALTERNATIVES = { 'Dropdown' => '0|1|2|3|4|5' }.freeze

*<code>def set_questionnaire_parameters</code> in <code>questionnaires_controller.rb</code>
# Assigns corrresponding variables to questionnaire object.
def set_questionnaire_parameters(private_flag, display)
@questionnaire.private = private_flag
@questionnaire.name = params[:questionnaire][:name]
@questionnaire.instructor_id = session[:user].id
@questionnaire.min_question_score = params[:questionnaire][:min_question_score]
@questionnaire.max_question_score = params[:questionnaire][:max_question_score]
@questionnaire.type = params[:questionnaire][:type]
@questionnaire.display_type = display
@questionnaire.instruction_loc = Questionnaire::DEFAULT_QUESTIONNAIRE_URL
@questionnaire.save
end

*advice_controller.rb lines 57-64: keys && each_key methods return incorrectly, keys.each is proper syntax for this functionality
before:
unless params[:advice].nil?
params[:advice].keys do |advice_key|
# Updates the advice corresponding to the key
QuestionAdvice.update(advice_key, advice: params[:advice] [advice_key.to_sym] [:advice])
end
after:
unless params[:advice].nil?
params[:advice].keys.each do |advice_key|
# Updates the advice corresponding to the key
QuestionAdvice.update(advice_key, advice: params[:advice] [advice_key.to_sym] [:advice])
end

*Questionnaires_controller.rb lines 89-99: removal of create_questionnaires method due to it having identical functionality to create
The following method was deleted
def create_questionnaire
@questionnaire = Object.const_get(params[:questionnaire][:type]).new(questionnaire_params)
# Create Quiz content has been moved to Quiz Questionnaire Controller
if @questionnaire.type != 'QuizQuestionnaire' # checking if it is a quiz questionnaire
@questionnaire.instructor_id = Ta.get_my_instructor(session[:user].id) if session[:user].role.name == 'Teaching Assistant'
save

redirect_to controller: 'tree_display', action: 'list'
end
end

== Important links ==
'''Github''': https://github.com/Soultrekker21/expertiza 
'''Pull Request''': https://github.com/expertiza/expertiza/pull/2576 
'''Current VCL''': http://152.7.178.100:3000/ 

= Test Plan =
* Make sure that current proposed changes don't break existing test cases.
* Add unit test for newly proposed methods.
* Shift existing test cases for questionnaire_controller_spec.rb to question_controller.rb corresponding to methods that are being shifted

= Testing Implementation =
*Removal of create_questionnaire method test: Testing a non-existent method no longer necessary
*Aside from this we found that the questionnaire controller is already well tested
*Below is questionnaires_controller_spec.rb tests run with <code>bundle exec rspec</code>
[[File:E2326_questionnaires_test.jpg|1000px|center]]
*Below is questions_controller_spec.rb tests run with <code>bundle exec rspec</code>
[[File:E2326_questions_test.jpg|1000px|center]]
*Below is quiz_questionnaires_controller_spec.rb tests run with <code>bundle exec rspec</code>
[[File:E2326_quiz_questionnaries_test.jpg|1000px|center]]

File:E2326 quiz questionnaries test.jpg

2023-05-02T22:04:56Z

Jtdavis9:

CSC/ECE 517 Spring 2023 -E2326 Refactor questionnaires controller.rb

2023-05-02T21:56:00Z

Jtdavis9: /* All Changes Implemented */

__TOC__

= Background =

In Expertiza, Questionnaires are used primarily by instructors to create question forms/ reviews, etc. As a student, you might have seen them while performing reviews for peers. The questionnaire is the superclass for all kinds of questionnaires and rubrics—rubrics for evaluating submissions and teammate contributions and taking quizzes and surveys. All of these are subclasses of the Questionnaire. questionnaires_controller.rb is charged with creating, displaying, and managing Questionnaires. Because it is used so widely, malfunctions could cause bugs in many parts of the system; hence it is especially important that the code be clear. 
A Questionnaire can have multiple Questions as a part of it. These questionnaires can be used as a part of an assignment.
Also, we notice that the Questionnaire controller has a parent-child relationship with the QuizQuestionnaire controller. There are multiple types of questions that can be added to a questionnaire like Criterion, Dropdown, Checkbox, and TextArea to name a few. There are multiple type of Questionnaires like Review, MetaReview, Assignment etc. 
To sum it off, we can have many types of Questionnaires and each would have possibly multiple types of Questions. There can be numerous questionnaires of the same kind for a given Questionnaire type.

= Project Purpose =

In recent years, <code>questionnaires_controller.rb</code> has been refactored repeatedly. It is a lot clearer than it used to be. But rough edges still remain.
question.rb contains a large number of constants. It is not clear what they are used for.
There is a questionnaires_controller, but no questions_controller which is fully developed. The questionnaires_controller creates Question objects. 
This is not elegant, because a controller should only create objects of one type, the type of objects it controls.
In the present code we see that the <code>questionnaire_controller.rb</code> has multiple non CRUD methods that should have been in the <code>questions_controller.rb</code>. Our priority is to refactor these functions to the <code>questions_controller.rb</code> so that we maintain the Single responsibility in the SOLID principles. We also notice that there are 2 places where there is code that is repeating itself so we extract the common code into a helper function to make the code more DRY. 
The following functions that need refactoring-
* <code>def delete_questions(questionnaire_id)</code> Move to questions_controller
* <code>def question_params</code> Move to questions_controller?
* <code>def save_all_questions</code> Move to questions_controller
* <code>def save_new_questions(questionnaire_id)</code> Move to questions_controller
* <code>def save_questions(questionnaire_id)</code> Move to questions_controller

We also noticed that there were many places that used constants like the text file are size or the default min and max scores being hard coded. This makes the code not suitable for extension as any change would need to be refactored in multiple places and break code easily. 
Thus we plan to modify the <code> models/questions.rb</code> and add the constants there so that all files related to questions can have access to these constants. This would make the refactoring process very easy.
 

= Design Pattern Used =

There are multiple types of questions created based on the parameters passed by the user to the controller. This led to two things, Objects being created dynamically which might not be of the acceptable kind, and code injection, as we are using unsanitized user input.

We have decided to use the '''Factory design''' pattern instead where we create a type of Questionnaire based on the type of parameters passed. Hence instead of multiple if/else code blocks to check the kind of input passed, we use a factory method to create and return the Questionnaire/Question object. Additionally, Instead of using if/else checks in the factory method, we opt for using a Map to match the input param to an Object type which further cleaned our code.

We employed '''SOLID''' principles, and shifted code that pertained to Questions from Questionnaire controller to Questions controller. This helps us better adhere to the '''Single Responsibility''' principle. We also shifted the common code out of the Questionnaire controller to helper classes to make the code more '''DRY'''.

= All Changes Implemented =

{| class="wikitable" style="width: 100%;
!  #  !! File/Function !! Change Proposed !! Rationale !! Commit Link
|-
|1
|method <code>add_new_questions()</code>
|Store the used default value to some kind of constant in <code>questions.rb</code>, so that they can be changed without refactoring code
|There are many literals used in the code and can change in future, so it is better to separate them
|[https://github.com/SoulTrekker21/expertiza/commit/4721039e523505e2c07f0691d8a7d42af2dc87ba Commit]
|-
|2
|files<code>question_helper.rb</code> and <code>questionnaire_helper.rb</code>
|Add a factory method that creates respective object based on the parameter that is being passed
|We use factory method in a helper file as that removes the code for creating an object from inside a function that just uses those objects
|[https://github.com/SoulTrekker21/expertiza/commit/4721039e523505e2c07f0691d8a7d42af2dc87ba Commit]
|-
|3
|File <code>questionnaires_controller.rb</code>
|Move <code>save_new_questions()</code>
|To be consistent with SOLID principle, these methods belong in questions_controller.rb
|[https://github.com/SoulTrekker21/expertiza/commit/9d3a846d862cfe4661c858e98902876cfe789a64 Commit]
|-
|4
|File <code>questionnaires_controller.rb</code>
|Move <code>delete_questions()</code>
|To be consistent with SOLID principle, these methods belong in questions_controller.rb
|[https://github.com/SoulTrekker21/expertiza/commit/669fc8e47c7a81223d48748f785af10b359f942e Commit]
|-
|5
|File <code>questionnaires_controller.rb</code>
|Move <code>save_questions()</code>
|To be consistent with SOLID principle, these methods belong in questions_controller.rb
|[https://github.com/SoulTrekker21/expertiza/commit/4edd184930109b920f7cf714b10dd22fd62c1f41 Commit]
|-
|6
|file <code>questionnaires_controller.rb</code> and <code>questions_controller.rb</code>
|Refactor or add more comments to the following vague method names
|There are confusing function names like <code>save_all_questions() </code> , <code>save_new_questions() </code> , <code>save_questions() </code> and <code>save() </code> and the use of each cannot be easily understood
|[https://github.com/SoulTrekker21/expertiza/commit/9edb52d359a1e341985259829bc7b4ddd224663a Commit]
|-
|7
|method <code>save_questions()</code>
|Change <code>params[:question].keys.each</code> to <code>params[:question].each_key</code>
| This will improve the performance and uses a single enumerator
|[https://github.com/Soultrekker21/expertiza/commit/690f8afe526d79c5811dcfbbc5dad826c1fd2c7c Commit]
|-
|8
|method <code>create()</code>
|Remove parenthesis around function call on line 72
|Unnecessary parenthesis
|[https://github.com/SoulTrekker21/expertiza/commit/4721039e523505e2c07f0691d8a7d42af2dc87ba Commit]
|-
|9
|method <code>create()</code> in <code>questionnaire_controller.rb</code>
|Move code related to creation of tree node to different function name <code>create_tree_node()</code>
|The create method is very big and this would help in readability and code re-usability
|[https://github.com/SoulTrekker21/expertiza/commit/4721039e523505e2c07f0691d8a7d42af2dc87ba Commit]
|-
|10
|method <code>questionnaire_helper()</code>
|Remove the usage of if/else or switch case and use a Hash Map for questionnaire factory object creation
|We want to make our code more modular and extendable. Any change can be easily added to Hash Map rather than adding if else statements.
|[https://github.com/SoulTrekker21/expertiza/commit/a24b18cb1702188a4d04483de8ca407920fef775 Commit]
|-
|11
|method <code>question_helper()</code>
|Remove the usage of if/else or switch case and use a Hash Map for questions factory object creation
|We want to make our code more modular and extendable. Any change can be easily added to Hash Map rather than adding if else statements.
|[https://github.com/SoulTrekker21/expertiza/commit/a24b18cb1702188a4d04483de8ca407920fef775 Commit]
|-
|12
|controller <code>advice_controller.rb</code>
|There needs to be a <code>.each</code> after they keys are enumerated
|The Edit/View advice button was not working on the view where we create a questionnaire and new questions
|[https://github.com/SoulTrekker21/expertiza/commit/690f8afe526d79c5811dcfbbc5dad826c1fd2c7c Commit]
|-
|13
|file <code>routes.rb</code>
|Add some routes so that all the new methods in <code>questions_controller.rb</code> can work
|Newly added functions don't have a predefined route and it must be specified
|[https://github.com/SoulTrekker21/expertiza/commit/843a12bce18bf441c8cb7e5b2a04d905bb155418 Commit]
|-
|14
|file <code>routes.rb</code>
|Add some routes so that the delete functionality works for questionnaires
|The existing code had the functionality implemented but the route was not configured that rendered an error
|[https://github.com/SoulTrekker21/expertiza/commit/6647464ad7c478ad7cd6ba27beecdee2cfa492f9 Commit]
|-
|15
|controller <code>quiz_questionnaire_controller.rb()</code>
|There are multiple redirects to functions in the <code>questionnaire_controller.rb</code>
|As some functions have been moved to the <code>questions_controller.rb</code> they need to be refactored.
|[https://github.com/SoulTrekker21/expertiza/commit/7b875fbcbc0bc23db2d14280945f09c4cc968223 Commit]
|}

= Sample of Code Changes =
*Questionnaire Factory where the object is created with switch case using a HashMap in <code>questionnaire_helper.rb</code>
#Map type to questionnaire
QUESTIONNAIRE_MAP = {
'ReviewQuestionnaire' => ReviewQuestionnaire,
'MetareviewQuestionnaire' => MetareviewQuestionnaire,
'AuthorFeedbackQuestionnaire' => AuthorFeedbackQuestionnaire,
'TeammateReviewQuestionnaire' => TeammateReviewQuestionnaire,
'AssignmentSurveyQuestionnaire' => AssignmentSurveyQuestionnaire,
'SurveyQuestionnaire' => SurveyQuestionnaire,
'GlobalSurveyQuestionnaire' => GlobalSurveyQuestionnaire,
'CourseSurveyQuestionnaire' => CourseSurveyQuestionnaire,
'BookmarkRatingQuestionnaire' => BookmarkRatingQuestionnaire,
'QuizQuestionnaire' => QuizQuestionnaire
}.freeze

# factory method to create the appropriate questionnaire based on the type
def questionnaire_factory(type)
questionnaire = QUESTIONNAIRE_MAP[type]
if questionnaire.nil?
flash[:error] = 'Error: Undefined Questionnaire'
else
questionnaire.new
end
end

*Question Factory where the questions object is created using a HashMap in <code>question_helper.rb</code>
module QuestionHelper
# Maps type to question
QUESTION_MAP = {
'Criterion' => Criterion,
'Scale' => Scale,
'Cake' => Cake,
'Dropdown' => Dropdown,
'Checkbox' => Checkbox,
'TextArea' => TextArea,
'TextField' => TextField,
'UploadFile' => UploadFile,
'SectionHeader' => SectionHeader,
'TableHeader' => TableHeader,
'ColumnHeader' => ColumnHeader
}.freeze

# factory method to create the appropriate question based on the type
def question_factory(type, questionnaire_id, seq)
question_class = QUESTION_MAP[type]
if question_class.nil?
flash[:error] = 'Error: Undefined Question'
else
question_class.create(txt: '', questionnaire_id: questionnaire_id, seq: seq, type: type, break_before: true)
end
end
end

*<code>models/question.rb</code> that shows how the constants are used to store the parameters that would be used for object creation
# Class variables - used questionnaires_controller.rb to set the parameters for a question.
MAX_LABEL = 'Strongly agree'.freeze
MIN_LABEL = 'Strongly disagree'.freeze
SIZES = { 'Criterion' => '50, 3', 'Cake' => '50, 3', 'TextArea' => '60, 5', 'TextField' => '30' }.freeze
ALTERNATIVES = { 'Dropdown' => '0|1|2|3|4|5' }.freeze

*<code>def set_questionnaire_parameters</code> in <code>questionnaires_controller.rb</code>
# Assigns corrresponding variables to questionnaire object.
def set_questionnaire_parameters(private_flag, display)
@questionnaire.private = private_flag
@questionnaire.name = params[:questionnaire][:name]
@questionnaire.instructor_id = session[:user].id
@questionnaire.min_question_score = params[:questionnaire][:min_question_score]
@questionnaire.max_question_score = params[:questionnaire][:max_question_score]
@questionnaire.type = params[:questionnaire][:type]
@questionnaire.display_type = display
@questionnaire.instruction_loc = Questionnaire::DEFAULT_QUESTIONNAIRE_URL
@questionnaire.save
end

*advice_controller.rb lines 57-64: keys && each_key methods return incorrectly, keys.each is proper syntax for this functionality
before:
unless params[:advice].nil?
params[:advice].keys do |advice_key|
# Updates the advice corresponding to the key
QuestionAdvice.update(advice_key, advice: params[:advice] [advice_key.to_sym] [:advice])
end
after:
unless params[:advice].nil?
params[:advice].keys.each do |advice_key|
# Updates the advice corresponding to the key
QuestionAdvice.update(advice_key, advice: params[:advice] [advice_key.to_sym] [:advice])
end

*Questionnaires_controller.rb lines 89-99: removal of create_questionnaires method due to it having identical functionality to create
The following method was deleted
def create_questionnaire
@questionnaire = Object.const_get(params[:questionnaire][:type]).new(questionnaire_params)
# Create Quiz content has been moved to Quiz Questionnaire Controller
if @questionnaire.type != 'QuizQuestionnaire' # checking if it is a quiz questionnaire
@questionnaire.instructor_id = Ta.get_my_instructor(session[:user].id) if session[:user].role.name == 'Teaching Assistant'
save

redirect_to controller: 'tree_display', action: 'list'
end
end

== Important links ==
Github: https://github.com/Soultrekker21/expertiza
Pull Request: https://github.com/expertiza/expertiza/pull/2576

= Test Plan =
* Make sure that current proposed changes don't break existing test cases.
* Add unit test for newly proposed methods.
* Shift existing test cases for questionnaire_controller_spec.rb to question_controller.rb corresponding to methods that are being shifted

= Testing Implementation =
*Removal of create_questionnaire method test: Testing a non-existent method no longer necessary
*Aside from this we found that the questionnaire controller is already well tested
*Below is questionnaires_controller_spec.rb tests run with <code>bundle exec rspec</code>
[[File:E2326_questionnaires_test.jpg|1000px|center]]
*Below is questions_controller_spec.rb tests run with <code>bundle exec rspec</code>
[[File:E2326_questions_test.jpg|1000px|center]]

CSC/ECE 517 Spring 2023 -E2326 Refactor questionnaires controller.rb

2023-05-02T21:55:38Z

Jtdavis9: Fixed links

__TOC__

= Background =

In Expertiza, Questionnaires are used primarily by instructors to create question forms/ reviews, etc. As a student, you might have seen them while performing reviews for peers. The questionnaire is the superclass for all kinds of questionnaires and rubrics—rubrics for evaluating submissions and teammate contributions and taking quizzes and surveys. All of these are subclasses of the Questionnaire. questionnaires_controller.rb is charged with creating, displaying, and managing Questionnaires. Because it is used so widely, malfunctions could cause bugs in many parts of the system; hence it is especially important that the code be clear. 
A Questionnaire can have multiple Questions as a part of it. These questionnaires can be used as a part of an assignment.
Also, we notice that the Questionnaire controller has a parent-child relationship with the QuizQuestionnaire controller. There are multiple types of questions that can be added to a questionnaire like Criterion, Dropdown, Checkbox, and TextArea to name a few. There are multiple type of Questionnaires like Review, MetaReview, Assignment etc. 
To sum it off, we can have many types of Questionnaires and each would have possibly multiple types of Questions. There can be numerous questionnaires of the same kind for a given Questionnaire type.

= Project Purpose =

In recent years, <code>questionnaires_controller.rb</code> has been refactored repeatedly. It is a lot clearer than it used to be. But rough edges still remain.
question.rb contains a large number of constants. It is not clear what they are used for.
There is a questionnaires_controller, but no questions_controller which is fully developed. The questionnaires_controller creates Question objects. 
This is not elegant, because a controller should only create objects of one type, the type of objects it controls.
In the present code we see that the <code>questionnaire_controller.rb</code> has multiple non CRUD methods that should have been in the <code>questions_controller.rb</code>. Our priority is to refactor these functions to the <code>questions_controller.rb</code> so that we maintain the Single responsibility in the SOLID principles. We also notice that there are 2 places where there is code that is repeating itself so we extract the common code into a helper function to make the code more DRY. 
The following functions that need refactoring-
* <code>def delete_questions(questionnaire_id)</code> Move to questions_controller
* <code>def question_params</code> Move to questions_controller?
* <code>def save_all_questions</code> Move to questions_controller
* <code>def save_new_questions(questionnaire_id)</code> Move to questions_controller
* <code>def save_questions(questionnaire_id)</code> Move to questions_controller

We also noticed that there were many places that used constants like the text file are size or the default min and max scores being hard coded. This makes the code not suitable for extension as any change would need to be refactored in multiple places and break code easily. 
Thus we plan to modify the <code> models/questions.rb</code> and add the constants there so that all files related to questions can have access to these constants. This would make the refactoring process very easy.
 

= Design Pattern Used =

There are multiple types of questions created based on the parameters passed by the user to the controller. This led to two things, Objects being created dynamically which might not be of the acceptable kind, and code injection, as we are using unsanitized user input.

We have decided to use the '''Factory design''' pattern instead where we create a type of Questionnaire based on the type of parameters passed. Hence instead of multiple if/else code blocks to check the kind of input passed, we use a factory method to create and return the Questionnaire/Question object. Additionally, Instead of using if/else checks in the factory method, we opt for using a Map to match the input param to an Object type which further cleaned our code.

We employed '''SOLID''' principles, and shifted code that pertained to Questions from Questionnaire controller to Questions controller. This helps us better adhere to the '''Single Responsibility''' principle. We also shifted the common code out of the Questionnaire controller to helper classes to make the code more '''DRY'''.

= All Changes Implemented =

{| class="wikitable" style="width: 100%;
!  #  !! File/Function !! Change Proposed !! Rationale !! Commit Link
|-
|1
|method <code>add_new_questions()</code>
|Store the used default value to some kind of constant in <code>questions.rb</code>, so that they can be changed without refactoring code
|There are many literals used in the code and can change in future, so it is better to separate them
|[https://github.com/SoulTrekker21/expertiza/commit/4721039e523505e2c07f0691d8a7d42af2dc87ba Commit]
|-
|2
|files<code>question_helper.rb</code> and <code>questionnaire_helper.rb</code>
|Add a factory method that creates respective object based on the parameter that is being passed
|We use factory method in a helper file as that removes the code for creating an object from inside a function that just uses those objects
|[https://github.com/SoulTrekker21/expertiza/commit/4721039e523505e2c07f0691d8a7d42af2dc87ba Commit]
|-
|3
|File <code>questionnaires_controller.rb</code>
|Move <code>save_new_questions()</code>
|To be consistent with SOLID principle, these methods belong in questions_controller.rb
|[https://github.com/SoulTrekker21/expertiza/commit/9d3a846d862cfe4661c858e98902876cfe789a64 Commit]
|-
|4
|File <code>questionnaires_controller.rb</code>
|Move <code>delete_questions()</code>
|To be consistent with SOLID principle, these methods belong in questions_controller.rb
|[https://github.com/SoulTrekker21/expertiza/commit/669fc8e47c7a81223d48748f785af10b359f942e Commit]
|-
|5
|File <code>questionnaires_controller.rb</code>
|Move <code>save_questions()</code>
|To be consistent with SOLID principle, these methods belong in questions_controller.rb
|[https://github.com/SoulTrekker21/expertiza/commit/4edd184930109b920f7cf714b10dd22fd62c1f41 Commit]
|-
|6
|file <code>questionnaires_controller.rb</code> and <code>questions_controller.rb</code>
|Refactor or add more comments to the following vague method names
|There are confusing function names like <code>save_all_questions() </code> , <code>save_new_questions() </code> , <code>save_questions() </code> and <code>save() </code> and the use of each cannot be easily understood
|[https://github.com/SoulTrekker21/expertiza/commit/9edb52d359a1e341985259829bc7b4ddd224663a Commit]
|-
|7
|method <code>save_questions()</code>
|Change <code>params[:question].keys.each</code> to <code>params[:question].each_key</code>
| This will improve the performance and uses a single enumerator
|[https://github.com/Soultrekker21/expertiza/commit/690f8afe526d79c5811dcfbbc5dad826c1fd2c7c Commit]
|-
|8
|method <code>create()</code>
|Remove parenthesis around function call on line 72
|Unnecessary parenthesis
|[https://github.com/SoulTrekker21/expertiza/commit/4721039e523505e2c07f0691d8a7d42af2dc87ba Commit]
|-
|9
|method <code>create()</code> in <code>questionnaire_controller.rb</code>
|Move code related to creation of tree node to different function name <code>create_tree_node()</code>
|The create method is very big and this would help in readability and code re-usability
|[https://github.com/SoulTrekker21/expertiza/commit/4721039e523505e2c07f0691d8a7d42af2dc87ba Commit]
|-
|10
|method <code>questionnaire_helper()</code>
|Remove the usage of if/else or switch case and use a Hash Map for questionnaire factory object creation
|We want to make our code more modular and extendable. Any change can be easily added to Hash Map rather than adding if else statements.
|[https://github.com/SoulTrekker21/expertiza/commit/a24b18cb1702188a4d04483de8ca407920fef775 Commit]
|-
|11
|method <code>question_helper()</code>
|Remove the usage of if/else or switch case and use a Hash Map for questions factory object creation
|We want to make our code more modular and extendable. Any change can be easily added to Hash Map rather than adding if else statements.
|[https://github.com/SoulTrekker21/expertiza/commit/a24b18cb1702188a4d04483de8ca407920fef775 Commit]
|-
|12
|controller <code>advice_controller.rb</code>
|There needs to be a <code>.each</code> after they keys are enumerated
|The Edit/View advice button was not working on the view where we create a questionnaire and new questions
|[https://github.com/SoulTrekker21/expertiza/commit/690f8afe526d79c5811dcfbbc5dad826c1fd2c7c Commit]
|-
|13
|file <code>routes.rb</code>
|Add some routes so that all the new methods in <code>questions_controller.rb</code> can work
|Newly added functions don't have a predefined route and it must be specified
|[https://github.com/SoulTrekker21/expertiza/commit/843a12bce18bf441c8cb7e5b2a04d905bb155418 Commit]
|-
|14
|file <code>routes.rb</code>
|Add some routes so that the delete functionality works for questionnaires
|The existing code had the functionality implemented but the route was not configured that rendered an error
|[https://github.com/SoulTrekker21/expertiza/commit/6647464ad7c478ad7cd6ba27beecdee2cfa492f9 Commit]
|-
|15
|controller <code>quiz_questionnaire_controller.rb()</code>
|There are multiple redirects to functions in the <code>questionnaire_controller.rb</code>
|As some functions have been moved to the <code>questions_controller.rb</code> they need to be refactored.
|[Commit link https://github.com/SoulTrekker21/expertiza/commit/7b875fbcbc0bc23db2d14280945f09c4cc968223 commit]
|}

= Sample of Code Changes =
*Questionnaire Factory where the object is created with switch case using a HashMap in <code>questionnaire_helper.rb</code>
#Map type to questionnaire
QUESTIONNAIRE_MAP = {
'ReviewQuestionnaire' => ReviewQuestionnaire,
'MetareviewQuestionnaire' => MetareviewQuestionnaire,
'AuthorFeedbackQuestionnaire' => AuthorFeedbackQuestionnaire,
'TeammateReviewQuestionnaire' => TeammateReviewQuestionnaire,
'AssignmentSurveyQuestionnaire' => AssignmentSurveyQuestionnaire,
'SurveyQuestionnaire' => SurveyQuestionnaire,
'GlobalSurveyQuestionnaire' => GlobalSurveyQuestionnaire,
'CourseSurveyQuestionnaire' => CourseSurveyQuestionnaire,
'BookmarkRatingQuestionnaire' => BookmarkRatingQuestionnaire,
'QuizQuestionnaire' => QuizQuestionnaire
}.freeze

# factory method to create the appropriate questionnaire based on the type
def questionnaire_factory(type)
questionnaire = QUESTIONNAIRE_MAP[type]
if questionnaire.nil?
flash[:error] = 'Error: Undefined Questionnaire'
else
questionnaire.new
end
end

*Question Factory where the questions object is created using a HashMap in <code>question_helper.rb</code>
module QuestionHelper
# Maps type to question
QUESTION_MAP = {
'Criterion' => Criterion,
'Scale' => Scale,
'Cake' => Cake,
'Dropdown' => Dropdown,
'Checkbox' => Checkbox,
'TextArea' => TextArea,
'TextField' => TextField,
'UploadFile' => UploadFile,
'SectionHeader' => SectionHeader,
'TableHeader' => TableHeader,
'ColumnHeader' => ColumnHeader
}.freeze

# factory method to create the appropriate question based on the type
def question_factory(type, questionnaire_id, seq)
question_class = QUESTION_MAP[type]
if question_class.nil?
flash[:error] = 'Error: Undefined Question'
else
question_class.create(txt: '', questionnaire_id: questionnaire_id, seq: seq, type: type, break_before: true)
end
end
end

*<code>models/question.rb</code> that shows how the constants are used to store the parameters that would be used for object creation
# Class variables - used questionnaires_controller.rb to set the parameters for a question.
MAX_LABEL = 'Strongly agree'.freeze
MIN_LABEL = 'Strongly disagree'.freeze
SIZES = { 'Criterion' => '50, 3', 'Cake' => '50, 3', 'TextArea' => '60, 5', 'TextField' => '30' }.freeze
ALTERNATIVES = { 'Dropdown' => '0|1|2|3|4|5' }.freeze

*<code>def set_questionnaire_parameters</code> in <code>questionnaires_controller.rb</code>
# Assigns corrresponding variables to questionnaire object.
def set_questionnaire_parameters(private_flag, display)
@questionnaire.private = private_flag
@questionnaire.name = params[:questionnaire][:name]
@questionnaire.instructor_id = session[:user].id
@questionnaire.min_question_score = params[:questionnaire][:min_question_score]
@questionnaire.max_question_score = params[:questionnaire][:max_question_score]
@questionnaire.type = params[:questionnaire][:type]
@questionnaire.display_type = display
@questionnaire.instruction_loc = Questionnaire::DEFAULT_QUESTIONNAIRE_URL
@questionnaire.save
end

*advice_controller.rb lines 57-64: keys && each_key methods return incorrectly, keys.each is proper syntax for this functionality
before:
unless params[:advice].nil?
params[:advice].keys do |advice_key|
# Updates the advice corresponding to the key
QuestionAdvice.update(advice_key, advice: params[:advice] [advice_key.to_sym] [:advice])
end
after:
unless params[:advice].nil?
params[:advice].keys.each do |advice_key|
# Updates the advice corresponding to the key
QuestionAdvice.update(advice_key, advice: params[:advice] [advice_key.to_sym] [:advice])
end

*Questionnaires_controller.rb lines 89-99: removal of create_questionnaires method due to it having identical functionality to create
The following method was deleted
def create_questionnaire
@questionnaire = Object.const_get(params[:questionnaire][:type]).new(questionnaire_params)
# Create Quiz content has been moved to Quiz Questionnaire Controller
if @questionnaire.type != 'QuizQuestionnaire' # checking if it is a quiz questionnaire
@questionnaire.instructor_id = Ta.get_my_instructor(session[:user].id) if session[:user].role.name == 'Teaching Assistant'
save

redirect_to controller: 'tree_display', action: 'list'
end
end

== Important links ==
Github: https://github.com/Soultrekker21/expertiza
Pull Request: https://github.com/expertiza/expertiza/pull/2576

= Test Plan =
* Make sure that current proposed changes don't break existing test cases.
* Add unit test for newly proposed methods.
* Shift existing test cases for questionnaire_controller_spec.rb to question_controller.rb corresponding to methods that are being shifted

= Testing Implementation =
*Removal of create_questionnaire method test: Testing a non-existent method no longer necessary
*Aside from this we found that the questionnaire controller is already well tested
*Below is questionnaires_controller_spec.rb tests run with <code>bundle exec rspec</code>
[[File:E2326_questionnaires_test.jpg|1000px|center]]
*Below is questions_controller_spec.rb tests run with <code>bundle exec rspec</code>
[[File:E2326_questions_test.jpg|1000px|center]]

CSC/ECE 517 Spring 2023 -E2326 Refactor questionnaires controller.rb

2023-05-02T19:31:14Z

Jtdavis9: Testing images

__TOC__

= Background =

In Expertiza, Questionnaire is the superclass for all kinds of questionnaires and rubrics—rubrics for evaluating submissions and teammate contributions, and taking quizzes and surveys. All of these are subclasses of Questionnaire. questionnaires_controller.rb is charged with creating, displaying, and managing Questionnaires. Because it is used so widely, malfunctions could cause bugs in many parts of the system; hence it is especially important that the code be clear.
We can see that a Questionnaire can have multiple Questions. These questionnaires can be used as a part of an assignment.
Also we notice that Questionnaire controler has a parent child relationship with the QuizQuestionnaire controller. There are multiple types of questions that can be added to a questionnaire like Criterion, Dropdown, Checkbox and TextArea to name a few.

= Project Purpose =

In recent years, <code>questionnaires_controller.rb</code> has been refactored repeatedly. It is a lot clearer than it used to be. But rough edges still remain.
question.rb contains a large number of constants. It is not clear what they are used for.
There is a questionnaires_controller, but no questions_controller which is fully developed. The questionnaires_controller creates Question objects. This is not elegant, because a controller should only create objects of one type, the type of objects it controls.
In the present code we see that the <code>questionnaire_controller.rb</code> has multiple non CRUD methods that should have been in the <code>questions_controller.rb</code>. Our priority is to refactor these functions to the <code>questions_controller.rb</code> so that we maintain the Single responsibility in the SOLID principles. We also notice that there are 2 places where there is code that is repeating itself so we extract the common code into a helper function to make the code more DRY.
the functions that need refactoring-
* <code>def delete_questions(questionnaire_id)</code> Move to questions_controller
* <code>def question_params</code> Move to questions_controller?
* <code>def save_all_questions</code> Move to questions_controller
* <code>def save_new_questions(questionnaire_id)</code> Move to questions_controller
* <code>def save_questions(questionnaire_id)</code> Move to questions_controller

We also noticed that there were many places that used constants like the text file are size or the default min and max scores being hard coded. This makes the code not suitable for extension as any change would need to be refactored in multiple places and break code easily. Thus we plan to modify the <code> models/questions.rb</code> and add the constants there so that all files related to questions can have access to these constants. This would make the refactoring process very easy.

= Design Pattern Used =

There are multiple types of questions created based on the parameters passed by the user to the controller. This led to two things, Objects being created dynamically which might not be of the acceptable kind, and code injection, as we are using unsanitized user input.

We have decided to use the '''Factory design''' pattern instead where we create a type of Questionnaire based on the type of parameters passed. Hence instead of multiple if/else code blocks to check the kind of input passed, we use a factory method to create and return the Questionnaire/Question object. Additionally, Instead of using if/else checks in the factory method, we opt for using a Map to match the input param to an Object type which further cleaned our code.

We employed '''SOLID''' principles, and shifted code that pertained to Questions from Questionnaire controller to Questions controller. This helps us better adhere to the '''Single Responsibility''' principle. We also shifted the common code out of the Questionnaire controller to helper classes to make the code more '''DRY'''.

= Proposed Design Changes =

{| class="wikitable" style="width: 100%;
!  #  !! File/Function !! Change Proposed !! Rationale !! Commit Link
|-
|1
|method <code>add_new_questions()</code>
|Store the used default value to some kind of constant in <code>questions.rb</code>, so that they can be changed without refactoring code
|There are many literals used in the code and can change in future, so it is better to separate them
|Will be added later
|-
|2
|File <code>questionnaires_controller.rb</code>
|Move save_all_questions(), delete_questions(), save_new_questions(), save_questions()
|To be consistent with SOLID principle, these methods belong in questions_controller.rb
|Will be added later
|-
|3
|file <code>questionnaires_controller.rb</code>
|Refactor following vague method names
|There are confusing function names like <code>save_all_questions() </code> , <code>save_new_questions() </code> , <code>save_questions() </code> and <code>save() </code>
|Will be added later
|-
|4
|method <code>save_questions()</code>
|Change <code>params[:question].keys.each</code> to <code>params[:question].each_key</code>
| This will improve the performance and uses a single enumerator
|Will be added late
|-
|5
|method <code>create()</code>
|Remove parenthesis around function call on line 72
|Unnecessary parenthesis
|Will be added later
|-
|5
|File <code>questionnaires_controller.rb</code>
|Refactor large methods
|Create, update, and delete are noticeably larger than most methods
|Will be added later
|-
|6
|method <code>save_all_questions()</code>
|Find a way to refactor the code by using more variables or Procs that can be reused in <code>questions_controller.rb</code>
|The process of saving all questions with 2 nested do blocks is difficult to understand
|Will be added later
|-
|6
|method create
|Move code related to creation of tree node to different function name <code>create_tree_node()</code>
|The create method is very big and this would help in readability and code reusability
|Will be added later
|-
|7
|method <code>questionnaire_helper()</code>
|Change if/else to switch case and add default error checking to helper
|Code will be cleaner and there will needs to be catch all because there are no default else conditions/ error checks in factories as of now.
|Will be added later
|-
|8
|method <code>question_helper()</code>
|Change if/else to switch case and add default error checking to helper
|Code will be cleaner and there will needs to be catch all because there are no default else conditions/ error checks in factories as of now.
|Will be added later
|-
|9
|method <code>question_helper()</code>
|Change if/else to switch case and add default error checking to helper
|Code will be cleaner and there will needs to be catch all because there are no default else conditions/ error checks in factories as of now.
|Will be added later
|-
|10
|method <code>question_helper()</code>
|Change if/else to switch case and add default error checking to helper
|Code will be cleaner and there will needs to be catch all because there are no default else conditions/ error checks in factories as of now.
|Will be added later
|-
|11
|method <code>question_helper()</code>
|Change if/else to switch case and add default error checking to helper
|Code will be cleaner and there will needs to be catch all because there are no default else conditions/ error checks in factories as of now.
|Will be added later
|-
|12
|controller <code>quiz_questionnaire_controller.rb()</code>
|There are multiple redirects to functions in the <code>questionnaire_controller.rb</code>
|As some functions have been moved to the <code>questions_controller.rb</code> they need to be refactored.
|[Commit link https://github.com/expertiza/expertiza/commit/7b875fbcbc0bc23db2d14280945f09c4cc968223 ]
|}

= Sample of Code Changes =
*Questionnaire Factory where the object is created with switch case using a HashMap in <code>questionnaire_helper.rb</code>
#Map type to questionnaire
QUESTIONNAIRE_MAP = {
'ReviewQuestionnaire' => ReviewQuestionnaire,
'MetareviewQuestionnaire' => MetareviewQuestionnaire,
'AuthorFeedbackQuestionnaire' => AuthorFeedbackQuestionnaire,
'TeammateReviewQuestionnaire' => TeammateReviewQuestionnaire,
'AssignmentSurveyQuestionnaire' => AssignmentSurveyQuestionnaire,
'SurveyQuestionnaire' => SurveyQuestionnaire,
'GlobalSurveyQuestionnaire' => GlobalSurveyQuestionnaire,
'CourseSurveyQuestionnaire' => CourseSurveyQuestionnaire,
'BookmarkRatingQuestionnaire' => BookmarkRatingQuestionnaire,
'QuizQuestionnaire' => QuizQuestionnaire
}.freeze

# factory method to create the appropriate questionnaire based on the type
def questionnaire_factory(type)
questionnaire = QUESTIONNAIRE_MAP[type]
if questionnaire.nil?
flash[:error] = 'Error: Undefined Questionnaire'
else
questionnaire.new
end
end

*Question Factory where the questions object is created using a HashMap in <code>question_helper.rb</code>
module QuestionHelper
# Maps type to question
QUESTION_MAP = {
'Criterion' => Criterion,
'Scale' => Scale,
'Cake' => Cake,
'Dropdown' => Dropdown,
'Checkbox' => Checkbox,
'TextArea' => TextArea,
'TextField' => TextField,
'UploadFile' => UploadFile,
'SectionHeader' => SectionHeader,
'TableHeader' => TableHeader,
'ColumnHeader' => ColumnHeader
}.freeze

# factory method to create the appropriate question based on the type
def question_factory(type, questionnaire_id, seq)
question_class = QUESTION_MAP[type]
if question_class.nil?
flash[:error] = 'Error: Undefined Question'
else
question_class.create(txt: '', questionnaire_id: questionnaire_id, seq: seq, type: type, break_before: true)
end
end
end

*<code>models/question.rb</code> that shows how the constants are used to store the parameters that would be used for object creation
# Class variables - used questionnaires_controller.rb to set the parameters for a question.
MAX_LABEL = 'Strongly agree'.freeze
MIN_LABEL = 'Strongly disagree'.freeze
SIZES = { 'Criterion' => '50, 3', 'Cake' => '50, 3', 'TextArea' => '60, 5', 'TextField' => '30' }.freeze
ALTERNATIVES = { 'Dropdown' => '0|1|2|3|4|5' }.freeze

*<code>def set_questionnaire_parameters</code> in <code>questionnaires_controller.rb</code>
# Assigns corrresponding variables to questionnaire object.
def set_questionnaire_parameters(private_flag, display)
@questionnaire.private = private_flag
@questionnaire.name = params[:questionnaire][:name]
@questionnaire.instructor_id = session[:user].id
@questionnaire.min_question_score = params[:questionnaire][:min_question_score]
@questionnaire.max_question_score = params[:questionnaire][:max_question_score]
@questionnaire.type = params[:questionnaire][:type]
@questionnaire.display_type = display
@questionnaire.instruction_loc = Questionnaire::DEFAULT_QUESTIONNAIRE_URL
@questionnaire.save
end

*advice_controller.rb lines 57-64: keys && each_key methods return incorrectly, keys.each is proper syntax for this functionality
before:
unless params[:advice].nil?
params[:advice].keys do |advice_key|
# Updates the advice corresponding to the key
QuestionAdvice.update(advice_key, advice: params[:advice] [advice_key.to_sym] [:advice])
end
after:
unless params[:advice].nil?
params[:advice].keys.each do |advice_key|
# Updates the advice corresponding to the key
QuestionAdvice.update(advice_key, advice: params[:advice] [advice_key.to_sym] [:advice])
end

*Questionnaires_controller.rb lines 89-99: removal of create_questionnaires method due to it having identical functionality to create
The following method was deleted
def create_questionnaire
@questionnaire = Object.const_get(params[:questionnaire][:type]).new(questionnaire_params)
# Create Quiz content has been moved to Quiz Questionnaire Controller
if @questionnaire.type != 'QuizQuestionnaire' # checking if it is a quiz questionnaire
@questionnaire.instructor_id = Ta.get_my_instructor(session[:user].id) if session[:user].role.name == 'Teaching Assistant'
save

redirect_to controller: 'tree_display', action: 'list'
end
end

== Important links ==
Github: https://github.com/Soultrekker21/expertiza
Pull Request: https://github.com/expertiza/expertiza/pull/2576

= Test Plan =
* Make sure that current proposed changes don't break existing test cases.
* Add unit test for newly proposed methods.
* Shift existing test cases for questionnaire_controller_spec.rb to question_controller.rb corresponding to methods that are being shifted

= Testing Implementation =
*Removal of create_questionnaire method test: Testing a non-existent method no longer necessary
*Aside from this we found that the questionnaire controller is already well tested
*Below is questionnaires_controller_spec.rb tests run with <code>bundle exec rspec</code>
[[File:E2326_questionnaires_test.jpg|1000px|center]]
*Below is questions_controller_spec.rb tests run with <code>bundle exec rspec</code>
[[File:E2326_questions_test.jpg|1000px|center]]

File:E2326 questions test.jpg

2023-05-02T19:26:03Z

Jtdavis9:

File:E2326 questionnaires test.jpg

2023-05-02T19:25:21Z

Jtdavis9:

CSC/ECE 517 Spring 2023 -E2326 Refactor questionnaires controller.rb

2023-05-02T18:59:20Z

Jtdavis9:

__TOC__

= Background =

In Expertiza, Questionnaire is the superclass for all kinds of questionnaires and rubrics—rubrics for evaluating submissions and teammate contributions, and taking quizzes and surveys. All of these are subclasses of Questionnaire. questionnaires_controller.rb is charged with creating, displaying, and managing Questionnaires. Because it is used so widely, malfunctions could cause bugs in many parts of the system; hence it is especially important that the code be clear.
We can see that a Questionnaire can have multiple Questions. These questionnaires can be used as a part of an assignment.
Also we notice that Questionnaire controler has a parent child relationship with the QuizQuestionnaire controller. There are multiple types of questions that can be added to a questionnaire like Criterion, Dropdown, Checkbox and TextArea to name a few.

= Project Purpose =

In recent years, <code>questionnaires_controller.rb</code> has been refactored repeatedly. It is a lot clearer than it used to be. But rough edges still remain.
question.rb contains a large number of constants. It is not clear what they are used for.
There is a questionnaires_controller, but no questions_controller which is fully developed. The questionnaires_controller creates Question objects. This is not elegant, because a controller should only create objects of one type, the type of objects it controls.
In the present code we see that the <code>questionnaire_controller.rb</code> has multiple non CRUD methods that should have been in the <code>questions_controller.rb</code>. Our priority is to refactor these functions to the <code>questions_controller.rb</code> so that we maintain the Single responsibility in the SOLID principles. We also notice that there are 2 places where there is code that is repeating itself so we extract the common code into a helper function to make the code more DRY.
the functions that need refactoring-
* <code>def delete_questions(questionnaire_id)</code> Move to questions_controller
* <code>def question_params</code> Move to questions_controller?
* <code>def save_all_questions</code> Move to questions_controller
* <code>def save_new_questions(questionnaire_id)</code> Move to questions_controller
* <code>def save_questions(questionnaire_id)</code> Move to questions_controller

We also noticed that there were many places that used constants like the text file are size or the default min and max scores being hard coded. This makes the code not suitable for extension as any change would need to be refactored in multiple places and break code easily. Thus we plan to modify the <code> models/questions.rb</code> and add the constants there so that all files related to questions can have access to these constants. This would make the refactoring process very easy.

= Design Pattern Used =

We have decided to used the Factory design pattern where we create a type of Questionnaire based on the type of parameters passed. The earlier code was using multiple if else statements to check the parameter passed and then create the required object.\\
In the current code we give a call to a helper function in the <file name> file and expect it to return the object based on the string parameter passed.

= Proposed Design Changes =

{| class="wikitable" style="width: 100%;
!  #  !! File/Function !! Change Proposed !! Rationale !! Commit Link
|-
|1
|method <code>add_new_questions()</code>
|Store the used default value to some kind of constant in <code>questions.rb</code>, so that they can be changed without refactoring code
|There are many literals used in the code and can change in future, so it is better to separate them
|Will be added later
|-
|2
|File <code>questionnaires_controller.rb</code>
|Move save_all_questions(), delete_questions(), save_new_questions(), save_questions()
|To be consistent with SOLID principle, these methods belong in questions_controller.rb
|Will be added later
|-
|3
|file <code>questionnaires_controller.rb</code>
|Refactor following vague method names
|There are confusing function names like <code>save_all_questions() </code> , <code>save_new_questions() </code> , <code>save_questions() </code> and <code>save() </code>
|Will be added later
|-
|4
|method <code>save_questions()</code>
|Change <code>params[:question].keys.each</code> to <code>params[:question].each_key</code>
| This will improve the performance and uses a single enumerator
|Will be added late
|-
|5
|method <code>create()</code>
|Remove parenthesis around function call on line 72
|Unnecessary parenthesis
|Will be added later
|-
|5
|File <code>questionnaires_controller.rb</code>
|Refactor large methods
|Create, update, and delete are noticeably larger than most methods
|Will be added later
|-
|6
|method <code>save_all_questions()</code>
|Find a way to refactor the code by using more variables or Procs that can be reused in <code>questions_controller.rb</code>
|The process of saving all questions with 2 nested do blocks is difficult to understand
|Will be added later
|-
|6
|method create
|Move code related to creation of tree node to different function name <code>create_tree_node()</code>
|The create method is very big and this would help in readability and code reusability
|Will be added later
|-
|7
|method <code>questionnaire_helper()</code>
|Change if/else to switch case and add default error checking to helper
|Code will be cleaner and there will needs to be catch all because there are no default else conditions/ error checks in factories as of now.
|Will be added later
|-
|8
|method <code>question_helper()</code>
|Change if/else to switch case and add default error checking to helper
|Code will be cleaner and there will needs to be catch all because there are no default else conditions/ error checks in factories as of now.
|Will be added later
|}

= Sample of Code Changes =
*Questionnaire Factory where the object is created with switch case using a HashMap in <code>questionnaire_helper.rb</code>
#Map type to questionnaire
QUESTIONNAIRE_MAP = {
'ReviewQuestionnaire' => ReviewQuestionnaire,
'MetareviewQuestionnaire' => MetareviewQuestionnaire,
'AuthorFeedbackQuestionnaire' => AuthorFeedbackQuestionnaire,
'TeammateReviewQuestionnaire' => TeammateReviewQuestionnaire,
'AssignmentSurveyQuestionnaire' => AssignmentSurveyQuestionnaire,
'SurveyQuestionnaire' => SurveyQuestionnaire,
'GlobalSurveyQuestionnaire' => GlobalSurveyQuestionnaire,
'CourseSurveyQuestionnaire' => CourseSurveyQuestionnaire,
'BookmarkRatingQuestionnaire' => BookmarkRatingQuestionnaire,
'QuizQuestionnaire' => QuizQuestionnaire
}.freeze

# factory method to create the appropriate questionnaire based on the type
def questionnaire_factory(type)
questionnaire = QUESTIONNAIRE_MAP[type]
if questionnaire.nil?
flash[:error] = 'Error: Undefined Questionnaire'
else
questionnaire.new
end
end

*Question Factory where the questions object is created using a HashMap in <code>question_helper.rb</code>
module QuestionHelper
# Maps type to question
QUESTION_MAP = {
'Criterion' => Criterion,
'Scale' => Scale,
'Cake' => Cake,
'Dropdown' => Dropdown,
'Checkbox' => Checkbox,
'TextArea' => TextArea,
'TextField' => TextField,
'UploadFile' => UploadFile,
'SectionHeader' => SectionHeader,
'TableHeader' => TableHeader,
'ColumnHeader' => ColumnHeader
}.freeze

# factory method to create the appropriate question based on the type
def question_factory(type, questionnaire_id, seq)
question_class = QUESTION_MAP[type]
if question_class.nil?
flash[:error] = 'Error: Undefined Question'
else
question_class.create(txt: '', questionnaire_id: questionnaire_id, seq: seq, type: type, break_before: true)
end
end
end

*<code>models/question.rb</code> that shows how the constants are used to store the parameters that would be used for object creation
# Class variables - used questionnaires_controller.rb to set the parameters for a question.
MAX_LABEL = 'Strongly agree'.freeze
MIN_LABEL = 'Strongly disagree'.freeze
SIZES = { 'Criterion' => '50, 3', 'Cake' => '50, 3', 'TextArea' => '60, 5', 'TextField' => '30' }.freeze
ALTERNATIVES = { 'Dropdown' => '0|1|2|3|4|5' }.freeze

*<code>def set_questionnaire_parameters</code> in <code>questionnaires_controller.rb</code>
# Assigns corrresponding variables to questionnaire object.
def set_questionnaire_parameters(private_flag, display)
@questionnaire.private = private_flag
@questionnaire.name = params[:questionnaire][:name]
@questionnaire.instructor_id = session[:user].id
@questionnaire.min_question_score = params[:questionnaire][:min_question_score]
@questionnaire.max_question_score = params[:questionnaire][:max_question_score]
@questionnaire.type = params[:questionnaire][:type]
@questionnaire.display_type = display
@questionnaire.instruction_loc = Questionnaire::DEFAULT_QUESTIONNAIRE_URL
@questionnaire.save
end

*advice_controller.rb lines 57-64: keys && each_key methods return incorrectly, keys.each is proper syntax for this functionality
before:
unless params[:advice].nil?
params[:advice].keys do |advice_key|
# Updates the advice corresponding to the key
QuestionAdvice.update(advice_key, advice: params[:advice] [advice_key.to_sym] [:advice])
end
after:
unless params[:advice].nil?
params[:advice].keys.each do |advice_key|
# Updates the advice corresponding to the key
QuestionAdvice.update(advice_key, advice: params[:advice] [advice_key.to_sym] [:advice])
end

*Questionnaires_controller.rb lines 89-99: removal of create_questionnaires method due to it having identical functionality to create
The following method was deleted
def create_questionnaire
@questionnaire = Object.const_get(params[:questionnaire][:type]).new(questionnaire_params)
# Create Quiz content has been moved to Quiz Questionnaire Controller
if @questionnaire.type != 'QuizQuestionnaire' # checking if it is a quiz questionnaire
@questionnaire.instructor_id = Ta.get_my_instructor(session[:user].id) if session[:user].role.name == 'Teaching Assistant'
save

redirect_to controller: 'tree_display', action: 'list'
end
end

== Important links ==
Github: https://github.com/Soultrekker21/expertiza
Pull Request: https://github.com/expertiza/expertiza/pull/2576

= Test Plan =
* Make sure that current proposed changes don't break existing test cases.
* Add unit test for newly proposed methods.
* Shift existing test cases for questionnaire_controller_spec.rb to question_controller.rb corresponding to methods that are being shifted

= Testing Implementation =
*Removal of create_questionnaire method test: Testing a non-existent method no longer necessary
*Aside from this we found that the questionnaire controller is already well tested

CSC/ECE 517 Spring 2023 -E2326 Refactor questionnaires controller.rb

2023-05-02T18:49:33Z

Jtdavis9: /* Sample of Code Changes */

__TOC__

= Background =

In Expertiza, Questionnaire is the superclass for all kinds of questionnaires and rubrics—rubrics for evaluating submissions and teammate contributions, and taking quizzes and surveys. All of these are subclasses of Questionnaire. questionnaires_controller.rb is charged with creating, displaying, and managing Questionnaires. Because it is used so widely, malfunctions could cause bugs in many parts of the system; hence it is especially important that the code be clear.
We can see that a Questionnaire can have multiple Questions. These questionnaires can be used as a part of an assignment.
Also we notice that Questionnaire controler has a parent child relationship with the QuizQuestionnaire controller. There are multiple types of questions that can be added to a questionnaire like Criterion, Dropdown, Checkbox and TextArea to name a few.

= Project Purpose =

In recent years, <code>questionnaires_controller.rb</code> has been refactored repeatedly. It is a lot clearer than it used to be. But rough edges still remain.
question.rb contains a large number of constants. It is not clear what they are used for.
There is a questionnaires_controller, but no questions_controller which is fully developed. The questionnaires_controller creates Question objects. This is not elegant, because a controller should only create objects of one type, the type of objects it controls.
In the present code we see that the <code>questionnaire_controller.rb</code> has multiple non CRUD methods that should have been in the <code>questions_controller.rb</code>. Our priority is to refactor these functions to the <code>questions_controller.rb</code> so that we maintain the Single responsibility in the SOLID principles. We also notice that there are 2 places where there is code that is repeating itself so we extract the common code into a helper function to make the code more DRY.
the functions that need refactoring-
* <code>def delete_questions(questionnaire_id)</code> Move to questions_controller
* <code>def question_params</code> Move to questions_controller?
* <code>def save_all_questions</code> Move to questions_controller
* <code>def save_new_questions(questionnaire_id)</code> Move to questions_controller
* <code>def save_questions(questionnaire_id)</code> Move to questions_controller

We also noticed that there were many places that used constants like the text file are size or the default min and max scores being hard coded. This makes the code not suitable for extension as any change would need to be refactored in multiple places and break code easily. Thus we plan to modify the <code> models/questions.rb</code> and add the constants there so that all files related to questions can have access to these constants. This would make the refactoring process very easy.

= Design Pattern Used =

We have decided to used the Factory design pattern where we create a type of Questionnaire based on the type of parameters passed. The earlier code was using multiple if else statements to check the parameter passed and then create the required object.\\
In the current code we give a call to a helper function in the <file name> file and expect it to return the object based on the string parameter passed.

= Proposed Design Changes =

{| class="wikitable" style="width: 100%;
!  #  !! File/Function !! Change Proposed !! Rationale !! Commit Link
|-
|1
|method <code>add_new_questions()</code>
|Store the used default value to some kind of constant in <code>questions.rb</code>, so that they can be changed without refactoring code
|There are many literals used in the code and can change in future, so it is better to separate them
|Will be added later
|-
|2
|File <code>questionnaires_controller.rb</code>
|Move save_all_questions(), delete_questions(), save_new_questions(), save_questions()
|To be consistent with SOLID principle, these methods belong in questions_controller.rb
|Will be added later
|-
|3
|file <code>questionnaires_controller.rb</code>
|Refactor following vague method names
|There are confusing function names like <code>save_all_questions() </code> , <code>save_new_questions() </code> , <code>save_questions() </code> and <code>save() </code>
|Will be added later
|-
|4
|method <code>save_questions()</code>
|Change <code>params[:question].keys.each</code> to <code>params[:question].each_key</code>
| This will improve the performance and uses a single enumerator
|Will be added late
|-
|5
|method <code>create()</code>
|Remove parenthesis around function call on line 72
|Unnecessary parenthesis
|Will be added later
|-
|5
|File <code>questionnaires_controller.rb</code>
|Refactor large methods
|Create, update, and delete are noticeably larger than most methods
|Will be added later
|-
|6
|method <code>save_all_questions()</code>
|Find a way to refactor the code by using more variables or Procs that can be reused in <code>questions_controller.rb</code>
|The process of saving all questions with 2 nested do blocks is difficult to understand
|Will be added later
|-
|6
|method create
|Move code related to creation of tree node to different function name <code>create_tree_node()</code>
|The create method is very big and this would help in readability and code reusability
|Will be added later
|-
|7
|method <code>questionnaire_helper()</code>
|Change if/else to switch case and add default error checking to helper
|Code will be cleaner and there will needs to be catch all because there are no default else conditions/ error checks in factories as of now.
|Will be added later
|-
|8
|method <code>question_helper()</code>
|Change if/else to switch case and add default error checking to helper
|Code will be cleaner and there will needs to be catch all because there are no default else conditions/ error checks in factories as of now.
|Will be added later
|}

= Sample of Code Changes =
*Questionnaire Factory where the object is created with switch case using a HashMap in <code>questionnaire_helper.rb</code>
#Map type to questionnaire
QUESTIONNAIRE_MAP = {
'ReviewQuestionnaire' => ReviewQuestionnaire,
'MetareviewQuestionnaire' => MetareviewQuestionnaire,
'AuthorFeedbackQuestionnaire' => AuthorFeedbackQuestionnaire,
'TeammateReviewQuestionnaire' => TeammateReviewQuestionnaire,
'AssignmentSurveyQuestionnaire' => AssignmentSurveyQuestionnaire,
'SurveyQuestionnaire' => SurveyQuestionnaire,
'GlobalSurveyQuestionnaire' => GlobalSurveyQuestionnaire,
'CourseSurveyQuestionnaire' => CourseSurveyQuestionnaire,
'BookmarkRatingQuestionnaire' => BookmarkRatingQuestionnaire,
'QuizQuestionnaire' => QuizQuestionnaire
}.freeze

# factory method to create the appropriate questionnaire based on the type
def questionnaire_factory(type)
questionnaire = QUESTIONNAIRE_MAP[type]
if questionnaire.nil?
flash[:error] = 'Error: Undefined Questionnaire'
else
questionnaire.new
end
end

*Question Factory where the questions object is created using a HashMap in <code>question_helper.rb</code>
module QuestionHelper
# Maps type to question
QUESTION_MAP = {
'Criterion' => Criterion,
'Scale' => Scale,
'Cake' => Cake,
'Dropdown' => Dropdown,
'Checkbox' => Checkbox,
'TextArea' => TextArea,
'TextField' => TextField,
'UploadFile' => UploadFile,
'SectionHeader' => SectionHeader,
'TableHeader' => TableHeader,
'ColumnHeader' => ColumnHeader
}.freeze

# factory method to create the appropriate question based on the type
def question_factory(type, questionnaire_id, seq)
question_class = QUESTION_MAP[type]
if question_class.nil?
flash[:error] = 'Error: Undefined Question'
else
question_class.create(txt: '', questionnaire_id: questionnaire_id, seq: seq, type: type, break_before: true)
end
end
end

*<code>models/question.rb</code> that shows how the constants are used to store the parameters that would be used for object creation
# Class variables - used questionnaires_controller.rb to set the parameters for a question.
MAX_LABEL = 'Strongly agree'.freeze
MIN_LABEL = 'Strongly disagree'.freeze
SIZES = { 'Criterion' => '50, 3', 'Cake' => '50, 3', 'TextArea' => '60, 5', 'TextField' => '30' }.freeze
ALTERNATIVES = { 'Dropdown' => '0|1|2|3|4|5' }.freeze

*<code>def set_questionnaire_parameters</code> in <code>questionnaires_controller.rb</code>
# Assigns corrresponding variables to questionnaire object.
def set_questionnaire_parameters(private_flag, display)
@questionnaire.private = private_flag
@questionnaire.name = params[:questionnaire][:name]
@questionnaire.instructor_id = session[:user].id
@questionnaire.min_question_score = params[:questionnaire][:min_question_score]
@questionnaire.max_question_score = params[:questionnaire][:max_question_score]
@questionnaire.type = params[:questionnaire][:type]
@questionnaire.display_type = display
@questionnaire.instruction_loc = Questionnaire::DEFAULT_QUESTIONNAIRE_URL
@questionnaire.save
end

*advice_controller.rb lines 57-64: keys && each_key methods return incorrectly, keys.each is proper syntax for this functionality
before:
unless params[:advice].nil?
params[:advice].keys do |advice_key|
# Updates the advice corresponding to the key
QuestionAdvice.update(advice_key, advice: params[:advice] [advice_key.to_sym] [:advice])
end
after:
unless params[:advice].nil?
params[:advice].keys.each do |advice_key|
# Updates the advice corresponding to the key
QuestionAdvice.update(advice_key, advice: params[:advice] [advice_key.to_sym] [:advice])
end

*Questionnaires_controller.rb lines 89-99: removal of create_questionnaires method due to it having identical functionality to create
The following method was deleted
def create_questionnaire
@questionnaire = Object.const_get(params[:questionnaire][:type]).new(questionnaire_params)
# Create Quiz content has been moved to Quiz Questionnaire Controller
if @questionnaire.type != 'QuizQuestionnaire' # checking if it is a quiz questionnaire
@questionnaire.instructor_id = Ta.get_my_instructor(session[:user].id) if session[:user].role.name == 'Teaching Assistant'
save

redirect_to controller: 'tree_display', action: 'list'
end
end

=== Important links ===
Github: https://github.com/Soultrekker21/expertiza
Pull Request: https://github.com/expertiza/expertiza/pull/2576

= Test Plan =
* Make sure that current proposed changes don't break existing test cases.
* Add unit test for newly proposed methods.
* Shift existing test cases for questionnaire_controller_spec.rb to question_controller.rb corresponding to methods that are being shifted

CSC/ECE 517 Spring 2023 -E2326 Refactor questionnaires controller.rb

2023-05-02T18:48:19Z

Jtdavis9: /* Sample of Code Changes */

__TOC__

= Background =

In Expertiza, Questionnaire is the superclass for all kinds of questionnaires and rubrics—rubrics for evaluating submissions and teammate contributions, and taking quizzes and surveys. All of these are subclasses of Questionnaire. questionnaires_controller.rb is charged with creating, displaying, and managing Questionnaires. Because it is used so widely, malfunctions could cause bugs in many parts of the system; hence it is especially important that the code be clear.
We can see that a Questionnaire can have multiple Questions. These questionnaires can be used as a part of an assignment.
Also we notice that Questionnaire controler has a parent child relationship with the QuizQuestionnaire controller. There are multiple types of questions that can be added to a questionnaire like Criterion, Dropdown, Checkbox and TextArea to name a few.

= Project Purpose =

In recent years, <code>questionnaires_controller.rb</code> has been refactored repeatedly. It is a lot clearer than it used to be. But rough edges still remain.
question.rb contains a large number of constants. It is not clear what they are used for.
There is a questionnaires_controller, but no questions_controller which is fully developed. The questionnaires_controller creates Question objects. This is not elegant, because a controller should only create objects of one type, the type of objects it controls.
In the present code we see that the <code>questionnaire_controller.rb</code> has multiple non CRUD methods that should have been in the <code>questions_controller.rb</code>. Our priority is to refactor these functions to the <code>questions_controller.rb</code> so that we maintain the Single responsibility in the SOLID principles. We also notice that there are 2 places where there is code that is repeating itself so we extract the common code into a helper function to make the code more DRY.
the functions that need refactoring-
* <code>def delete_questions(questionnaire_id)</code> Move to questions_controller
* <code>def question_params</code> Move to questions_controller?
* <code>def save_all_questions</code> Move to questions_controller
* <code>def save_new_questions(questionnaire_id)</code> Move to questions_controller
* <code>def save_questions(questionnaire_id)</code> Move to questions_controller

We also noticed that there were many places that used constants like the text file are size or the default min and max scores being hard coded. This makes the code not suitable for extension as any change would need to be refactored in multiple places and break code easily. Thus we plan to modify the <code> models/questions.rb</code> and add the constants there so that all files related to questions can have access to these constants. This would make the refactoring process very easy.

= Design Pattern Used =

We have decided to used the Factory design pattern where we create a type of Questionnaire based on the type of parameters passed. The earlier code was using multiple if else statements to check the parameter passed and then create the required object.\\
In the current code we give a call to a helper function in the <file name> file and expect it to return the object based on the string parameter passed.

= Proposed Design Changes =

{| class="wikitable" style="width: 100%;
!  #  !! File/Function !! Change Proposed !! Rationale !! Commit Link
|-
|1
|method <code>add_new_questions()</code>
|Store the used default value to some kind of constant in <code>questions.rb</code>, so that they can be changed without refactoring code
|There are many literals used in the code and can change in future, so it is better to separate them
|Will be added later
|-
|2
|File <code>questionnaires_controller.rb</code>
|Move save_all_questions(), delete_questions(), save_new_questions(), save_questions()
|To be consistent with SOLID principle, these methods belong in questions_controller.rb
|Will be added later
|-
|3
|file <code>questionnaires_controller.rb</code>
|Refactor following vague method names
|There are confusing function names like <code>save_all_questions() </code> , <code>save_new_questions() </code> , <code>save_questions() </code> and <code>save() </code>
|Will be added later
|-
|4
|method <code>save_questions()</code>
|Change <code>params[:question].keys.each</code> to <code>params[:question].each_key</code>
| This will improve the performance and uses a single enumerator
|Will be added late
|-
|5
|method <code>create()</code>
|Remove parenthesis around function call on line 72
|Unnecessary parenthesis
|Will be added later
|-
|5
|File <code>questionnaires_controller.rb</code>
|Refactor large methods
|Create, update, and delete are noticeably larger than most methods
|Will be added later
|-
|6
|method <code>save_all_questions()</code>
|Find a way to refactor the code by using more variables or Procs that can be reused in <code>questions_controller.rb</code>
|The process of saving all questions with 2 nested do blocks is difficult to understand
|Will be added later
|-
|6
|method create
|Move code related to creation of tree node to different function name <code>create_tree_node()</code>
|The create method is very big and this would help in readability and code reusability
|Will be added later
|-
|7
|method <code>questionnaire_helper()</code>
|Change if/else to switch case and add default error checking to helper
|Code will be cleaner and there will needs to be catch all because there are no default else conditions/ error checks in factories as of now.
|Will be added later
|-
|8
|method <code>question_helper()</code>
|Change if/else to switch case and add default error checking to helper
|Code will be cleaner and there will needs to be catch all because there are no default else conditions/ error checks in factories as of now.
|Will be added later
|}

= Sample of Code Changes =
*Questionnaire Factory where the object is created with switch case using a HashMap in <code>questionnaire_helper.rb</code>
#Map type to questionnaire
QUESTIONNAIRE_MAP = {
'ReviewQuestionnaire' => ReviewQuestionnaire,
'MetareviewQuestionnaire' => MetareviewQuestionnaire,
'AuthorFeedbackQuestionnaire' => AuthorFeedbackQuestionnaire,
'TeammateReviewQuestionnaire' => TeammateReviewQuestionnaire,
'AssignmentSurveyQuestionnaire' => AssignmentSurveyQuestionnaire,
'SurveyQuestionnaire' => SurveyQuestionnaire,
'GlobalSurveyQuestionnaire' => GlobalSurveyQuestionnaire,
'CourseSurveyQuestionnaire' => CourseSurveyQuestionnaire,
'BookmarkRatingQuestionnaire' => BookmarkRatingQuestionnaire,
'QuizQuestionnaire' => QuizQuestionnaire
}.freeze

# factory method to create the appropriate questionnaire based on the type
def questionnaire_factory(type)
questionnaire = QUESTIONNAIRE_MAP[type]
if questionnaire.nil?
flash[:error] = 'Error: Undefined Questionnaire'
else
questionnaire.new
end
end

*Question Factory where the questions object is created using a HashMap in <code>question_helper.rb</code>
module QuestionHelper
# Maps type to question
QUESTION_MAP = {
'Criterion' => Criterion,
'Scale' => Scale,
'Cake' => Cake,
'Dropdown' => Dropdown,
'Checkbox' => Checkbox,
'TextArea' => TextArea,
'TextField' => TextField,
'UploadFile' => UploadFile,
'SectionHeader' => SectionHeader,
'TableHeader' => TableHeader,
'ColumnHeader' => ColumnHeader
}.freeze

# factory method to create the appropriate question based on the type
def question_factory(type, questionnaire_id, seq)
question_class = QUESTION_MAP[type]
if question_class.nil?
flash[:error] = 'Error: Undefined Question'
else
question_class.create(txt: '', questionnaire_id: questionnaire_id, seq: seq, type: type, break_before: true)
end
end
end

*<code>models/question.rb</code> that shows how the constants are used to store the parameters that would be used for object creation
# Class variables - used questionnaires_controller.rb to set the parameters for a question.
MAX_LABEL = 'Strongly agree'.freeze
MIN_LABEL = 'Strongly disagree'.freeze
SIZES = { 'Criterion' => '50, 3', 'Cake' => '50, 3', 'TextArea' => '60, 5', 'TextField' => '30' }.freeze
ALTERNATIVES = { 'Dropdown' => '0|1|2|3|4|5' }.freeze

*<code>def set_questionnaire_parameters</code> in <code>questionnaires_controller.rb</code>
# Assigns corrresponding variables to questionnaire object.
def set_questionnaire_parameters(private_flag, display)
@questionnaire.private = private_flag
@questionnaire.name = params[:questionnaire][:name]
@questionnaire.instructor_id = session[:user].id
@questionnaire.min_question_score = params[:questionnaire][:min_question_score]
@questionnaire.max_question_score = params[:questionnaire][:max_question_score]
@questionnaire.type = params[:questionnaire][:type]
@questionnaire.display_type = display
@questionnaire.instruction_loc = Questionnaire::DEFAULT_QUESTIONNAIRE_URL
@questionnaire.save
end

*advice_controller.rb lines 57-64: keys && each_key methods return incorrectly, keys.each is proper syntax for this functionality
before:
unless params[:advice].nil?
params[:advice].keys do |advice_key|
# Updates the advice corresponding to the key
QuestionAdvice.update(advice_key, advice: params[:advice] [advice_key.to_sym] [:advice])
end
after:
unless params[:advice].nil?
params[:advice].keys.each do |advice_key|
# Updates the advice corresponding to the key
QuestionAdvice.update(advice_key, advice: params[:advice] [advice_key.to_sym] [:advice])
end

*Questionnaires_controller.rb lines 89-99: removal of create_questionnaires method due to it having identical functionality to create
The following method was deleted
def create_questionnaire
@questionnaire = Object.const_get(params[:questionnaire][:type]).new(questionnaire_params)
# Create Quiz content has been moved to Quiz Questionnaire Controller
if @questionnaire.type != 'QuizQuestionnaire' # checking if it is a quiz questionnaire
@questionnaire.instructor_id = Ta.get_my_instructor(session[:user].id) if session[:user].role.name == 'Teaching Assistant'
save

redirect_to controller: 'tree_display', action: 'list'
end
end
=== Important links ===
Github: https://github.com/Soultrekker21/expertiza
Pull Request: https://github.com/expertiza/expertiza/pull/2576

= Test Plan =
* Make sure that current proposed changes don't break existing test cases.
* Add unit test for newly proposed methods.
* Shift existing test cases for questionnaire_controller_spec.rb to question_controller.rb corresponding to methods that are being shifted

CSC/ECE 517 Spring 2023 -E2326 Refactor questionnaires controller.rb

2023-05-02T18:48:00Z

Jtdavis9: code changes added

__TOC__

= Background =

In Expertiza, Questionnaire is the superclass for all kinds of questionnaires and rubrics—rubrics for evaluating submissions and teammate contributions, and taking quizzes and surveys. All of these are subclasses of Questionnaire. questionnaires_controller.rb is charged with creating, displaying, and managing Questionnaires. Because it is used so widely, malfunctions could cause bugs in many parts of the system; hence it is especially important that the code be clear.
We can see that a Questionnaire can have multiple Questions. These questionnaires can be used as a part of an assignment.
Also we notice that Questionnaire controler has a parent child relationship with the QuizQuestionnaire controller. There are multiple types of questions that can be added to a questionnaire like Criterion, Dropdown, Checkbox and TextArea to name a few.

= Project Purpose =

In recent years, <code>questionnaires_controller.rb</code> has been refactored repeatedly. It is a lot clearer than it used to be. But rough edges still remain.
question.rb contains a large number of constants. It is not clear what they are used for.
There is a questionnaires_controller, but no questions_controller which is fully developed. The questionnaires_controller creates Question objects. This is not elegant, because a controller should only create objects of one type, the type of objects it controls.
In the present code we see that the <code>questionnaire_controller.rb</code> has multiple non CRUD methods that should have been in the <code>questions_controller.rb</code>. Our priority is to refactor these functions to the <code>questions_controller.rb</code> so that we maintain the Single responsibility in the SOLID principles. We also notice that there are 2 places where there is code that is repeating itself so we extract the common code into a helper function to make the code more DRY.
the functions that need refactoring-
* <code>def delete_questions(questionnaire_id)</code> Move to questions_controller
* <code>def question_params</code> Move to questions_controller?
* <code>def save_all_questions</code> Move to questions_controller
* <code>def save_new_questions(questionnaire_id)</code> Move to questions_controller
* <code>def save_questions(questionnaire_id)</code> Move to questions_controller

We also noticed that there were many places that used constants like the text file are size or the default min and max scores being hard coded. This makes the code not suitable for extension as any change would need to be refactored in multiple places and break code easily. Thus we plan to modify the <code> models/questions.rb</code> and add the constants there so that all files related to questions can have access to these constants. This would make the refactoring process very easy.

= Design Pattern Used =

We have decided to used the Factory design pattern where we create a type of Questionnaire based on the type of parameters passed. The earlier code was using multiple if else statements to check the parameter passed and then create the required object.\\
In the current code we give a call to a helper function in the <file name> file and expect it to return the object based on the string parameter passed.

= Proposed Design Changes =

{| class="wikitable" style="width: 100%;
!  #  !! File/Function !! Change Proposed !! Rationale !! Commit Link
|-
|1
|method <code>add_new_questions()</code>
|Store the used default value to some kind of constant in <code>questions.rb</code>, so that they can be changed without refactoring code
|There are many literals used in the code and can change in future, so it is better to separate them
|Will be added later
|-
|2
|File <code>questionnaires_controller.rb</code>
|Move save_all_questions(), delete_questions(), save_new_questions(), save_questions()
|To be consistent with SOLID principle, these methods belong in questions_controller.rb
|Will be added later
|-
|3
|file <code>questionnaires_controller.rb</code>
|Refactor following vague method names
|There are confusing function names like <code>save_all_questions() </code> , <code>save_new_questions() </code> , <code>save_questions() </code> and <code>save() </code>
|Will be added later
|-
|4
|method <code>save_questions()</code>
|Change <code>params[:question].keys.each</code> to <code>params[:question].each_key</code>
| This will improve the performance and uses a single enumerator
|Will be added late
|-
|5
|method <code>create()</code>
|Remove parenthesis around function call on line 72
|Unnecessary parenthesis
|Will be added later
|-
|5
|File <code>questionnaires_controller.rb</code>
|Refactor large methods
|Create, update, and delete are noticeably larger than most methods
|Will be added later
|-
|6
|method <code>save_all_questions()</code>
|Find a way to refactor the code by using more variables or Procs that can be reused in <code>questions_controller.rb</code>
|The process of saving all questions with 2 nested do blocks is difficult to understand
|Will be added later
|-
|6
|method create
|Move code related to creation of tree node to different function name <code>create_tree_node()</code>
|The create method is very big and this would help in readability and code reusability
|Will be added later
|-
|7
|method <code>questionnaire_helper()</code>
|Change if/else to switch case and add default error checking to helper
|Code will be cleaner and there will needs to be catch all because there are no default else conditions/ error checks in factories as of now.
|Will be added later
|-
|8
|method <code>question_helper()</code>
|Change if/else to switch case and add default error checking to helper
|Code will be cleaner and there will needs to be catch all because there are no default else conditions/ error checks in factories as of now.
|Will be added later
|}

= Sample of Code Changes =
*Questionnaire Factory where the object is created with switch case using a HashMap in <code>questionnaire_helper.rb</code>
#Map type to questionnaire
QUESTIONNAIRE_MAP = {
'ReviewQuestionnaire' => ReviewQuestionnaire,
'MetareviewQuestionnaire' => MetareviewQuestionnaire,
'AuthorFeedbackQuestionnaire' => AuthorFeedbackQuestionnaire,
'TeammateReviewQuestionnaire' => TeammateReviewQuestionnaire,
'AssignmentSurveyQuestionnaire' => AssignmentSurveyQuestionnaire,
'SurveyQuestionnaire' => SurveyQuestionnaire,
'GlobalSurveyQuestionnaire' => GlobalSurveyQuestionnaire,
'CourseSurveyQuestionnaire' => CourseSurveyQuestionnaire,
'BookmarkRatingQuestionnaire' => BookmarkRatingQuestionnaire,
'QuizQuestionnaire' => QuizQuestionnaire
}.freeze

# factory method to create the appropriate questionnaire based on the type
def questionnaire_factory(type)
questionnaire = QUESTIONNAIRE_MAP[type]
if questionnaire.nil?
flash[:error] = 'Error: Undefined Questionnaire'
else
questionnaire.new
end
end

*Question Factory where the questions object is created using a HashMap in <code>question_helper.rb</code>
module QuestionHelper
# Maps type to question
QUESTION_MAP = {
'Criterion' => Criterion,
'Scale' => Scale,
'Cake' => Cake,
'Dropdown' => Dropdown,
'Checkbox' => Checkbox,
'TextArea' => TextArea,
'TextField' => TextField,
'UploadFile' => UploadFile,
'SectionHeader' => SectionHeader,
'TableHeader' => TableHeader,
'ColumnHeader' => ColumnHeader
}.freeze

# factory method to create the appropriate question based on the type
def question_factory(type, questionnaire_id, seq)
question_class = QUESTION_MAP[type]
if question_class.nil?
flash[:error] = 'Error: Undefined Question'
else
question_class.create(txt: '', questionnaire_id: questionnaire_id, seq: seq, type: type, break_before: true)
end
end
end

*<code>models/question.rb</code> that shows how the constants are used to store the parameters that would be used for object creation
# Class variables - used questionnaires_controller.rb to set the parameters for a question.
MAX_LABEL = 'Strongly agree'.freeze
MIN_LABEL = 'Strongly disagree'.freeze
SIZES = { 'Criterion' => '50, 3', 'Cake' => '50, 3', 'TextArea' => '60, 5', 'TextField' => '30' }.freeze
ALTERNATIVES = { 'Dropdown' => '0|1|2|3|4|5' }.freeze

*<code>def set_questionnaire_parameters</code> in <code>questionnaires_controller.rb</code>
# Assigns corrresponding variables to questionnaire object.
def set_questionnaire_parameters(private_flag, display)
@questionnaire.private = private_flag
@questionnaire.name = params[:questionnaire][:name]
@questionnaire.instructor_id = session[:user].id
@questionnaire.min_question_score = params[:questionnaire][:min_question_score]
@questionnaire.max_question_score = params[:questionnaire][:max_question_score]
@questionnaire.type = params[:questionnaire][:type]
@questionnaire.display_type = display
@questionnaire.instruction_loc = Questionnaire::DEFAULT_QUESTIONNAIRE_URL
@questionnaire.save
end

*advice_controller.rb lines 57-64: keys && each_key methods return incorrectly, keys.each is proper syntax for this functionality
before:
unless params[:advice].nil?
params[:advice].keys do |advice_key|
# Updates the advice corresponding to the key
QuestionAdvice.update(advice_key, advice: params[:advice] [advice_key.to_sym] [:advice])
end
after:
unless params[:advice].nil?
params[:advice].keys.each do |advice_key|
# Updates the advice corresponding to the key
QuestionAdvice.update(advice_key, advice: params[:advice] [advice_key.to_sym] [:advice])
end

*Questionnaires_controller.rb lines 89-99: removal of create_questionnaires method due to it having identical functionality to create
The following method was deleted
def create_questionnaire
@questionnaire = Object.const_get(params[:questionnaire][:type]).new(questionnaire_params)
# Create Quiz content has been moved to Quiz Questionnaire Controller
if @questionnaire.type != 'QuizQuestionnaire' # checking if it is a quiz questionnaire
@questionnaire.instructor_id = Ta.get_my_instructor(session[:user].id) if session[:user].role.name == 'Teaching Assistant'
save

redirect_to controller: 'tree_display', action: 'list'
end
end
=== Important links ===
Github: https://github.com/Soultrekker21/expertiza
Pull Request: https://github.com/expertiza/expertiza/pull/2576

= Test Plan =
* Make sure that current proposed changes don't break existing test cases.
* Add unit test for newly proposed methods.
* Shift existing test cases for questionnaire_controller_spec.rb to question_controller.rb corresponding to methods that are being shifted

CSC/ECE 517 Spring 2023 -E2326 Refactor questionnaires controller.rb

2023-05-02T18:38:56Z

Jtdavis9: code changes added

__TOC__

= Background =

In Expertiza, Questionnaire is the superclass for all kinds of questionnaires and rubrics—rubrics for evaluating submissions and teammate contributions, and taking quizzes and surveys. All of these are subclasses of Questionnaire. questionnaires_controller.rb is charged with creating, displaying, and managing Questionnaires. Because it is used so widely, malfunctions could cause bugs in many parts of the system; hence it is especially important that the code be clear.
We can see that a Questionnaire can have multiple Questions. These questionnaires can be used as a part of an assignment.
Also we notice that Questionnaire controler has a parent child relationship with the QuizQuestionnaire controller. There are multiple types of questions that can be added to a questionnaire like Criterion, Dropdown, Checkbox and TextArea to name a few.

= Project Purpose =

In recent years, <code>questionnaires_controller.rb</code> has been refactored repeatedly. It is a lot clearer than it used to be. But rough edges still remain.
question.rb contains a large number of constants. It is not clear what they are used for.
There is a questionnaires_controller, but no questions_controller which is fully developed. The questionnaires_controller creates Question objects. This is not elegant, because a controller should only create objects of one type, the type of objects it controls.
In the present code we see that the <code>questionnaire_controller.rb</code> has multiple non CRUD methods that should have been in the <code>questions_controller.rb</code>. Our priority is to refactor these functions to the <code>questions_controller.rb</code> so that we maintain the Single responsibility in the SOLID principles. We also notice that there are 2 places where there is code that is repeating itself so we extract the common code into a helper function to make the code more DRY.
the functions that need refactoring-
* <code>def delete_questions(questionnaire_id)</code> Move to questions_controller
* <code>def question_params</code> Move to questions_controller?
* <code>def save_all_questions</code> Move to questions_controller
* <code>def save_new_questions(questionnaire_id)</code> Move to questions_controller
* <code>def save_questions(questionnaire_id)</code> Move to questions_controller

We also noticed that there were many places that used constants like the text file are size or the default min and max scores being hard coded. This makes the code not suitable for extension as any change would need to be refactored in multiple places and break code easily. Thus we plan to modify the <code> models/questions.rb</code> and add the constants there so that all files related to questions can have access to these constants. This would make the refactoring process very easy.

= Design Pattern Used =

We have decided to used the Factory design pattern where we create a type of Questionnaire based on the type of parameters passed. The earlier code was using multiple if else statements to check the parameter passed and then create the required object.\\
In the current code we give a call to a helper function in the <file name> file and expect it to return the object based on the string parameter passed.

= Proposed Design Changes =

{| class="wikitable" style="width: 100%;
!  #  !! File/Function !! Change Proposed !! Rationale !! Commit Link
|-
|1
|method <code>add_new_questions()</code>
|Store the used default value to some kind of constant in <code>questions.rb</code>, so that they can be changed without refactoring code
|There are many literals used in the code and can change in future, so it is better to separate them
|Will be added later
|-
|2
|File <code>questionnaires_controller.rb</code>
|Move save_all_questions(), delete_questions(), save_new_questions(), save_questions()
|To be consistent with SOLID principle, these methods belong in questions_controller.rb
|Will be added later
|-
|3
|file <code>questionnaires_controller.rb</code>
|Refactor following vague method names
|There are confusing function names like <code>save_all_questions() </code> , <code>save_new_questions() </code> , <code>save_questions() </code> and <code>save() </code>
|Will be added later
|-
|4
|method <code>save_questions()</code>
|Change <code>params[:question].keys.each</code> to <code>params[:question].each_key</code>
| This will improve the performance and uses a single enumerator
|Will be added late
|-
|5
|method <code>create()</code>
|Remove parenthesis around function call on line 72
|Unnecessary parenthesis
|Will be added later
|-
|5
|File <code>questionnaires_controller.rb</code>
|Refactor large methods
|Create, update, and delete are noticeably larger than most methods
|Will be added later
|-
|6
|method <code>save_all_questions()</code>
|Find a way to refactor the code by using more variables or Procs that can be reused in <code>questions_controller.rb</code>
|The process of saving all questions with 2 nested do blocks is difficult to understand
|Will be added later
|-
|6
|method create
|Move code related to creation of tree node to different function name <code>create_tree_node()</code>
|The create method is very big and this would help in readability and code reusability
|Will be added later
|-
|7
|method <code>questionnaire_helper()</code>
|Change if/else to switch case and add default error checking to helper
|Code will be cleaner and there will needs to be catch all because there are no default else conditions/ error checks in factories as of now.
|Will be added later
|-
|8
|method <code>question_helper()</code>
|Change if/else to switch case and add default error checking to helper
|Code will be cleaner and there will needs to be catch all because there are no default else conditions/ error checks in factories as of now.
|Will be added later
|}

= Sample of Code Changes =
* Questionnaire Factory where the object is created with switch case using a HashMap in <code>questionnaire_helper.rb</code>

#Map type to questionnaire
QUESTIONNAIRE_MAP = {
'ReviewQuestionnaire' => ReviewQuestionnaire,
'MetareviewQuestionnaire' => MetareviewQuestionnaire,
'AuthorFeedbackQuestionnaire' => AuthorFeedbackQuestionnaire,
'TeammateReviewQuestionnaire' => TeammateReviewQuestionnaire,
'AssignmentSurveyQuestionnaire' => AssignmentSurveyQuestionnaire,
'SurveyQuestionnaire' => SurveyQuestionnaire,
'GlobalSurveyQuestionnaire' => GlobalSurveyQuestionnaire,
'CourseSurveyQuestionnaire' => CourseSurveyQuestionnaire,
'BookmarkRatingQuestionnaire' => BookmarkRatingQuestionnaire,
'QuizQuestionnaire' => QuizQuestionnaire
}.freeze

# factory method to create the appropriate questionnaire based on the type
def questionnaire_factory(type)
questionnaire = QUESTIONNAIRE_MAP[type]
if questionnaire.nil?
flash[:error] = 'Error: Undefined Questionnaire'
else
questionnaire.new
end
end

* Question Factory where the questions object is created using a HashMap in <code>question_helper.rb</code>

module QuestionHelper
# Maps type to question
QUESTION_MAP = {
'Criterion' => Criterion,
'Scale' => Scale,
'Cake' => Cake,
'Dropdown' => Dropdown,
'Checkbox' => Checkbox,
'TextArea' => TextArea,
'TextField' => TextField,
'UploadFile' => UploadFile,
'SectionHeader' => SectionHeader,
'TableHeader' => TableHeader,
'ColumnHeader' => ColumnHeader
}.freeze

# factory method to create the appropriate question based on the type
def question_factory(type, questionnaire_id, seq)
question_class = QUESTION_MAP[type]
if question_class.nil?
flash[:error] = 'Error: Undefined Question'
else
question_class.create(txt: '', questionnaire_id: questionnaire_id, seq: seq, type: type, break_before: true)
end
end
end

* <code>models/question.rb</code> that shows how the constants are used to store the parameters that would be used for object creation

# Class variables - used questionnaires_controller.rb to set the parameters for a question.
MAX_LABEL = 'Strongly agree'.freeze
MIN_LABEL = 'Strongly disagree'.freeze
SIZES = { 'Criterion' => '50, 3', 'Cake' => '50, 3', 'TextArea' => '60, 5', 'TextField' => '30' }.freeze
ALTERNATIVES = { 'Dropdown' => '0|1|2|3|4|5' }.freeze

* <code>def set_questionnaire_parameters</code> in <code>questionnaires_controller.rb</code>
# Assigns corrresponding variables to questionnaire object.
def set_questionnaire_parameters(private_flag, display)
@questionnaire.private = private_flag
@questionnaire.name = params[:questionnaire][:name]
@questionnaire.instructor_id = session[:user].id
@questionnaire.min_question_score = params[:questionnaire][:min_question_score]
@questionnaire.max_question_score = params[:questionnaire][:max_question_score]
@questionnaire.type = params[:questionnaire][:type]
@questionnaire.display_type = display
@questionnaire.instruction_loc = Questionnaire::DEFAULT_QUESTIONNAIRE_URL
@questionnaire.save
end

=== Important links ===
Github: https://github.com/Soultrekker21/expertiza
Pull Request: https://github.com/expertiza/expertiza/pull/2576

= Test Plan =
* Make sure that current proposed changes don't break existing test cases.
* Add unit test for newly proposed methods.
* Shift existing test cases for questionnaire_controller_spec.rb to question_controller.rb corresponding to methods that are being shifted

CSC/ECE 517 Spring 2023 -E2326 Refactor questionnaires controller.rb

2023-05-02T18:32:26Z

Jtdavis9:

CSC/ECE 517 Spring 2023 -E2326 Refactor questionnaires controller.rb

2023-04-09T01:17:03Z

Jtdavis9: /* Design */

CSC/ECE 517 Spring 2023 -E2326 Refactor questionnaires controller.rb

2023-04-09T01:12:58Z

Jtdavis9:

CSC/ECE 517 Spring 2023 -E2326 Refactor questionnaires controller.rb

2023-04-09T01:09:15Z

Jtdavis9:

CSC/ECE 517 Spring 2023 -E2326 Refactor questionnaires controller.rb

2023-04-09T01:09:04Z

Jtdavis9: /*added to table*/

CSC/ECE 517 Spring 2023 -E2326 Refactor questionnaires controller.rb

2023-04-09T01:04:25Z

Jtdavis9: /* Test Plan */

CSC/ECE 517 Spring 2023 -E2326 Refactor questionnaires controller.rb

2023-04-09T01:01:32Z

Jtdavis9:

CSC/ECE 517 Spring 2023 -E2326 Refactor questionnaires controller.rb

2023-04-09T00:42:36Z

Jtdavis9: /* Jacob */

CSC/ECE 517 Spring 2023 -E2326 Refactor questionnaires controller.rb

2023-04-08T23:09:50Z

Jtdavis9:

CSC/ECE 517 Spring 2023 -E2326 Refactor questionnaires controller.rb

2023-04-08T23:09:38Z

Jtdavis9:

CSC/ECE 517 Spring 2023 -E2326 Refactor questionnaires controller.rb

2023-04-08T22:24:52Z

Jtdavis9:

CSC/ECE 517 Spring 2023 -E2326 Refactor questionnaires controller.rb

2023-04-08T22:24:03Z

Jtdavis9:

CSC/ECE 517 Spring 2023 -E2326 Refactor questionnaires controller.rb

2023-04-08T22:23:24Z

Jtdavis9:

__TOC__

= Project Purpose =

In recent years, questionnaires_controller.rb has been refactored repeatedly. It is a lot clearer than it used to be. But rough edges still remain.
question.rb contains a large number of constants. It is not clear what they are used for.
There is a questionnaires_controller, but no questions_controller. The questionnaires_controller creates Question objects. This is not elegant, because a controller should only create objects of one type, the type of objects it controls.

= Background =

In Expertiza, Questionnaire is the superclass for all kinds of questionnaires and rubrics—rubrics for evaluating submissions and teammate contributions, and taking quizzes and surveys. All of these are subclasses of Questionnaire. questionnaires_controller.rb is charged with creating, displaying, and managing Questionnaires. Because it is used so widely, malfunctions could cause bugs in many parts of the system; hence it is especially important that the code be clear.

= Design =

=== Problems and planned changes ===
problems and planned changes

=== Important links ===
Github: https://github.com/Soultrekker21/expertiza

= Test Plan =
test1
test2

CSC/ECE 517 Spring 2023 -E2326 Refactor questionnaires controller.rb

2023-04-08T22:20:18Z

Jtdavis9:

CSC/ECE 517 Spring 2023 -E2326 Refactor questionnaires controller.rb

2023-04-08T22:19:51Z

Jtdavis9:

__TOC__
= Project Overview =

== Background ==

In Expertiza, Questionnaire is the superclass for all kinds of questionnaires and rubrics—rubrics for evaluating submissions and teammate contributions, and taking quizzes and surveys. All of these are subclasses of Questionnaire. questionnaires_controller.rb is charged with creating, displaying, and managing Questionnaires. Because it is used so widely, malfunctions could cause bugs in many parts of the system; hence it is especially important that the code be clear.

== Project Purpose ==

In recent years, questionnaires_controller.rb has been refactored repeatedly. It is a lot clearer than it used to be. But rough edges still remain.
question.rb contains a large number of constants. It is not clear what they are used for.
There is a questionnaires_controller, but no questions_controller. The questionnaires_controller creates Question objects. This is not elegant, because a controller should only create objects of one type, the type of objects it controls.

== Design ==

=== Problems and planned changes ===
problems and planned changes

== Important links ==
Github: https://github.com/Soultrekker21/expertiza

== Test Plan ==
test1
test2

CSC/ECE 517 Spring 2023 -E2326 Refactor questionnaires controller.rb

2023-04-08T22:10:54Z

Jtdavis9:

__TOC__
== PROJECT OVERVIEW ==

=== Background ===

In Expertiza, Questionnaire is the superclass for all kinds of questionnaires and rubrics—rubrics for evaluating submissions and teammate contributions, and taking quizzes and surveys. All of these are subclasses of Questionnaire. questionnaires_controller.rb is charged with creating, displaying, and managing Questionnaires. Because it is used so widely, malfunctions could cause bugs in many parts of the system; hence it is especially important that the code be clear.

In recent years, questionnaires_controller.rb has been refactored repeatedly. It is a lot clearer than it used to be. But rough edges still remain.
question.rb contains a large number of constants. It is not clear what they are used for.
There is a questionnaires_controller, but no questions_controller. The questionnaires_controller creates Question objects. This is not elegant, because a controller should only create objects of one type, the type of objects it controls.

=== Problem Statement ===

Problem statement

== Summary of Work Needed ==

=== Problems and planned changes ===
problems and planned changes

== Work Completed ==

=== Solutions to Problems ===
solution1
solution2
=== Testing ===
test1
test2
=== Important links ===
Github: https://github.com/Soultrekker21/expertiza

CSC/ECE 517 Spring 2023 -E2326 Refactor questionnaires controller.rb

2023-04-08T19:49:09Z

Jtdavis9:

CSC/ECE 517 Spring 2023 -E2326 Refactor questionnaires controller.rb

2023-04-08T19:48:03Z

Jtdavis9:

__TOC__
=== PROJECT OVERVIEW ===

== Background ==

In Expertiza, Questionnaire is the superclass for all kinds of questionnaires and rubrics—rubrics for evaluating submissions and teammate contributions, and taking quizzes and surveys. All of these are subclasses of Questionnaire. questionnaires_controller.rb is charged with creating, displaying, and managing Questionnaires. Because it is used so widely, malfunctions could cause bugs in many parts of the system; hence it is especially important that the code be clear.

In recent years, questionnaires_controller.rb has been refactored repeatedly. It is a lot clearer than it used to be. But rough edges still remain.
question.rb contains a large number of constants. It is not clear what they are used for.
There is a questionnaires_controller, but no questions_controller. The questionnaires_controller creates Question objects. This is not elegant, because a controller should only create objects of one type, the type of objects it controls.

== Problem Statement ==

Problem statement

=== Summary of Work Needed ===

== Problems and planned changes ==
problems and planned changes

=== Work Completed ===

== Solutions to Problems ==
solution1
solution2
== Testing ==
test1
test2
== Important links ==
Link to Github

CSC/ECE 517 Spring 2023 -E2326 Refactor questionnaires controller.rb

2023-04-08T19:45:19Z

Jtdavis9:

__TOC__
=== PROJECT OVERVIEW ===

== Background ==

In Expertiza, Questionnaire is the superclass for all kinds of questionnaires and rubrics—rubrics for evaluating submissions and teammate contributions, and taking quizzes and surveys. All of these are subclasses of Questionnaire. questionnaires_controller.rb is charged with creating, displaying, and managing Questionnaires. Because it is used so widely, malfunctions could cause bugs in many parts of the system; hence it is especially important that the code be clear.
In recent years, questionnaires_controller.rb has been refactored repeatedly. It is a lot clearer than it used to be. But rough edges still remain.
Please look at the comments I made on your code. I think it can be simplified considerably.
Is it really necessary to use so many instance variables? Would temporary variables suffice?
question.rb contains a large number of constants. It is not clear what they are used for.
I have also made several comments about how the code could be made clearer.
There is a questionnaires_controller, but no questions_controller. The questionnaires_controller creates Question objects. This is not elegant, because a controller should only create objects of one type, the type of objects it controls.

== Problem Statement ==

=== Summary of Work Needed ===

== Problems and planned changes ==

=== Work Completed ===

== Solutions to Problems ==

== Testing ==

== Important links ==

CSC/ECE 517 Spring 2023 -E2326 Refactor questionnaires controller.rb

2023-04-08T19:24:13Z

Jtdavis9: Created page with "=== PROJECT OVERVIEW ==="

=== PROJECT OVERVIEW ===

CSC/ECE 517 Spring 2023 - E2302

2023-03-28T03:13:24Z

Jtdavis9:

==E2302. Refactoring the Delayed Mailer and Scheduler==

__TOC__

== Expertiza Overview ==

===Background===

Expertiza is an open-source software written using Ruby on Rails which functions as a learning management software system. It has many different functions and abilities including the ability to create assignments, quizzes, assignment groups, and topics, and also a complete mechanism for providing peer reviews and feedback for other groups and other teammates.

''Sidekiq is a popular open-source gem for Ruby on Rails applications that provides a simple and efficient way to perform background processing for time-consuming tasks. It uses multi-threading to handle jobs asynchronously, allowing the main application to continue running while the background jobs are executed. Sidekiq provides a queue system that manages jobs and prioritizes them based on their urgency.'' The docs can be found here [https://sidekiq.org/about.html].

Sidekiq is used in Expertiza to schedule background tasks such as Plagiarism checks for Assignments (Similink) and the mail for assignment's due dates.

Based on our task prompt we understood that the create method of the assignment controller needs to be changed alongside the testing needs to be improved for sidekiq-related worker files. Also, the code should be commented more thoroughly and variable names changed to better reflect the context they are used for.

===Prior Work===

=== History: Previous projects ===

* E2253 [https://expertiza.csc.ncsu.edu/index.php/CSC/ECE_517_Fall_2022_-_E2253.Refactor_delayed_mailer.rb_and_scheduled_task.rb]
* E2144 [https://expertiza.csc.ncsu.edu/index.php/CSC/ECE_517_Fall_2021_-_E2144._Refactor_delayed_mailer_and_scheduled_task]
* E2273 [https://expertiza.csc.ncsu.edu/index.php/CSC/ECE_517_Fall_2022_-_E2273.Refactor_delayed_mailer.rb_and_scheduled_task.rb]

The latest Pull Request was not merged into the repository due to some failing tests and missing comments. 
Tests were failing in the following files-
* <code>spec/models/assignment_form_spec.rb</code>
* <code>spec/controllers/assignments_controller_spec.rb</code> 
We try to address those failing tests and add block comments before function definition wherever needed. 
Additional testing of some related files is also in the scope of the project.

=== Our Changes ===

Upon review of previous PRs, we found that the work was thorough, albeit a few test cases were failing, and comments and methods could be refactored better. Based on the prompts we got, we focussed on refactoring the create method of assignments controller, and assignments form, along with the refactoring of sidekiq-related files.

Below is the summary of all the changes as part of this PR.
==== Changes to <code>spec/controllers/auth_controller_spec.rb</code> ====
{| class="wikitable" style="width: 100%;
!  #  !! Change !! Rationale !! Commit Link
|-
|1
|Refactored the <code>create</code> method into multiple methods
| The current <code>create</code> method is long and have multiple complex references and if statements. Created separate helpers to simplify complex DB context
|[https://github.com/tusharkini/expertiza/commit/4c2f0e04e7e3ab10443af26d2868758b5c696b55 Commit]
|-
|2
|Used early return in case of errors
|We refactored the code such that the errors like the assignment already being present or the directory already being present were caught early in the code. It leads to cleaner code as well.
|[https://github.com/tusharkini/expertiza/commit/3c00648b7228eb0daf8a9afd30f8f3abfb44c2c4 Commit]
|-
|3
|Renamed variable names
|Some of the current variable names were ambiguous as to what they meant, <code>(cur_questionnaire, cur_due, etc.)</code>. Renamed those names to be less ambiguous and state their function.
|[https://github.com/tusharkini/expertiza/commit/4c2f0e04e7e3ab10443af26d2868758b5c696b55 Commit]
|-
|4
|Added comments to other helper methods
|Some functions had no comments which were needed.
|[https://github.com/tusharkini/expertiza/commit/b72febb7f2370d6ccb0c99939500f6d36178bf00 Commit]
|}

==== Changes to <code>app/mailers/mail_worker.rb</code> ====
{| class="wikitable" style="width: 100%;
!  #  !! Change !! Rationale !! Commit Link
|-
|1
|Removed inline comments and added block comments to the function perform
|Block comments before the function allows better readability and looks clean
|[https://github.com/tusharkini/expertiza/commit/b56ecca4db6e1784a3f92b4fa9619992c1d4dcb2 Commit]
|-
|2
|Moved the line to find the participants of a course inside the conditional block
|Earlier the list of participants of a course were found even when the <code>deadline_type</code> was <code>'drop_outstanding_reviews’</code>. We do find the list of emails when it is not <code>'drop_outstanding_reviews’</code> and send emails to them.
|[https://github.com/tusharkini/expertiza/commit/b56ecca4db6e1784a3f92b4fa9619992c1d4dcb2 Commit]
|-
|3
|Combined the multiple <code>attr_accessor</code>lines into a single line
|Better readability and shorter function code.
|[https://github.com/tusharkini/expertiza/commit/b56ecca4db6e1784a3f92b4fa9619992c1d4dcb2 Commit]
|-
|4
|@mail object is not used as it is not reused anywhere else
|Any temporary variable that is used once should be avoided as it takes up valuable resources.
|[https://github.com/tusharkini/expertiza/commit/b56ecca4db6e1784a3f92b4fa9619992c1d4dcb2 Commit]
|}

==== Changes to <code>spec/models/assignment_form_spec.rb</code> ====
{| class="wikitable" style="width: 100%;
!  #  !! Change !! Rationale !! Commit Link
|-
|1
| Replaced ‘mailers’ with ‘jobs’ in <code>add_to_delayed_queue</code> test
| Test was failing due to checking the wrong Sidekiq queue, currently <code>add_to_delayed_queue</code> adds to job queue NOT mailers queue
|[https://github.com/tusharkini/expertiza/commit/95dcdaaa36b837df3d580f3352b8504535cc7c2e Commit]
|}

==== Changes to <code>app/models/assignment_form.rb</code> ====
{| class="wikitable" style="width: 100%;
!  #  !! Change !! Rationale !! Commit Link
|-
|1
|Added block comments to methods and removed inline comments
|Better readability and looks clean
|[https://github.com/expertiza/expertiza/pull/2556/commits/975312c88bb65861b1b6f2b268dc8d80c15aa501 Commit]
|-
|2
|Created <code>default_assignment_questionaire</code> to handle creation of a default <code>assignment_questionnaire</code> that was originally in assignment questionnaire
|<code>assignment_questionnaire</code> originally not only searched for an <code>assignment_questionnaire</code> but created one if one matching type did not exist. Now the second functionality (creating a default <code>assignment_questionnaire</code>) is its own method.
|[https://github.com/expertiza/expertiza/pull/2556/commits/b784e829920059cbff412ad7768833e95469ff2b Commit]
|-
|3
|In line 58 of original code replaced <code>[][]</code> method of searching with <code>.dig</code>
|This particular line could throw errors and .dig is a safer way to search a table without the risk of getting nil errors
|[https://github.com/expertiza/expertiza/pull/2556/commits/975312c88bb65861b1b6f2b268dc8d80c15aa501 Commit]
|-
|4
|Remove the switch case and moved code out of conditional
|There was repetition of code in multiple cases of switch statement. Moved them out of the case so that they are executed after the if statements.
|[https://github.com/tusharkini/expertiza/commit/158bf7bc971a7bcfba7cf377cccdbdeef62cab40 Commit]
|}

==== Changes to <code>spec/sidekiq_mail_worker_spec.rb</code> ====
{| class="wikitable" style="width: 100%;
!  #  !! Change !! Rationale !! Commit Link
|-
|1
|Added test case for method <code>perform</code> in <code>mail_worker.rb</code>
|This test case was not implemented in the earlier Pull Request
|[https://github.com/tusharkini/expertiza/commit/8d78d40ea4e4035f95522e025fd08e220b8b9ce4 Commit]
|-
|2
|Added test case for method <code>find_participant_emails</code> in <code>mail_worker.rb</code>
|This test case was not implemented in the earlier Pull Request
|[https://github.com/tusharkini/expertiza/commit/190ca781681021f995a8a034a34bb8cede1733e3 Commit]
|-
|3
|Added test case for method <code>email_reminder</code> in <code>mail_worker.rb</code>
|This test case was not implemented in the earlier Pull Request
|[https://github.com/tusharkini/expertiza/commit/53b8cf281215aeeec857886deaa2fae7998eff47 Commit]

|}

=== Test Coverage for some related files===
==== <code>spec/models/assignment_form_spec.rb</code> ====
After making the changes to <code>assignment_form.rb </code> and <code>assignment_form_spec.rb </code> we ran the rspec tests with the command <code>bundle exec rspec spec/models/assignment_form_spec.rb </code>
[[File:Assignment_form_test.jpg|1000px]]

==== <code>spec/workers/sidekiq_mail_worker_spec.rb</code> ====
Automated testing of <code>sidekiq_mailer_worker_spec.rb </code> with the command <code>bundle exec rspec spec/workers/sidekiq_mailer_worker_spec.rb </code>
[[File:mailer_spec.jpg|1000px]]

==== <code>spec/controllers/auth_controller_spec.rb</code> ====
Automated testing of <code>assignments_controller_spec.rb </code> with the command <code>bundle exec rspec spec/controllers/assignment_form_spec.rb </code>
[[File:Assignment_controller_test.jpg|1000px]]

=== Testing / Test Plan ===
The enhancement is tested manually and through rspec tests.

VCL : http://152.7.178.223:8080/

Login: instructor_Lien

Password: password

To Test via UI:

1) Login to VCL with the above credentials

2) Click on Manage tab

3) Click on Assignments

4) click on + to add a new assignment

5) Create an assignment with a non-zero delay(this feature is to send a reminder email to participants of the course) in SimiCheck(this is the Plagiarism Checker that is being used) and Similarity Threshold to a non-zero.

6) Go to the due date and add a due date for any day after the current date and timestamp. Just be sure that you have atleast 'delay'(set above while creating the assignment) number of hours left for the deadline. Or else the jobs would not be scheduled properly.

7) Create the assignment

8) Go to the Assignment page and add participants (ex. 'Paul' & 'Dan' to name a few. You can query the database and find more Students in the test database) to the assignment.

9) Check the sidekiq dashboard [http://152.7.178.223:8080/sidekiq] page for queued jobs. Check the time left for the MailWorker that would be sending out the mails.

10) It would be under the scheduled tab and would have 'delay' number of hours left less that the actual time left for the deadline. This indicated that the Mailer jobs is queued in a perfect way to send out reminder emails.

=== Edge Cases ===

Error shown in the dashboard:

[[File: Functional_test_error.jpg|1000px]] 
1) Note: In previous refactoring changes it was noted that this error is to be expected and will not affect the real Expertiza. Error is due to Simicheck Plagiarism Checker not having access to certain features in the PR build. This is currently out of the scope of the project and is yet to be implemented in totality. Also the Scheduled MailWorker that sends the email out to user would fail because of lack of SMTP authentication in the test database and environment. It is present in the production environment to which we dont have access.

2) If setting up the environment yourself, the users table in expertiza_development DB would need to be updated and a Preferred Time needed to be added in already created users, for assignments to be saved.

=== Commented out Tests so that the Build passed ===
These are some tests that were failing and causing the build to fail. These tests that are commented out are completely un related to our project and fall way outside of our scope to go and resolve it. 
The files are- 
* ./spec/models/course_participant_spec.rb
*./spec/models/course_participant_spec.rb
*./spec/models/assignment_participant_spec.rb
*./spec/models/due_date_spec.rb

=== Changes to Code wrt CodeClimate ===
* Removing duplicate Code in files related to our project
* Make the lines smaller if number of characters is more than 80
* Remove any kind of trailing spaces in the files
* Indentation issues were fixed

=== Coverage Report in the Build of the Pull Request ===
[[File:E2302_coveralls.png|1000px]]

=== Unrelated Tests that are failing ===
[[File:E2302_Failing.jpg|1000px]]

File:E2302 Failing.jpg

2023-03-28T03:11:35Z

Jtdavis9:

File:Functional test error.jpg

2023-03-23T03:12:14Z

Jtdavis9: Jtdavis9 uploaded a new version of File:Functional test error.jpg

CSC/ECE 517 Spring 2023 - E2302

2023-03-23T03:08:51Z

Jtdavis9: /* Testing */

==E2302. Refactoring the Delayed Mailer and Scheduler==

__TOC__

== Expertiza Overview ==

===Background===

Expertiza is an open source software written using Ruby on Rails which functions as a learning management software system. It has man different functions and abilities including the ability to create assignments, quizzes, assignment groups and topics, and also a complete mechanism for providing peer reviews and feedback for other groups and other teammates.

===Prior Work===

=== History: Previous projects ===

* E2253 [https://expertiza.csc.ncsu.edu/index.php/CSC/ECE_517_Fall_2022_-_E2253.Refactor_delayed_mailer.rb_and_scheduled_task.rb]
* E2144 [https://expertiza.csc.ncsu.edu/index.php/CSC/ECE_517_Fall_2021_-_E2144._Refactor_delayed_mailer_and_scheduled_task]
* E2273 [https://expertiza.csc.ncsu.edu/index.php/CSC/ECE_517_Fall_2022_-_E2273.Refactor_delayed_mailer.rb_and_scheduled_task.rb]

The latest Pull Request was not merged into the repository due to some failing tests and missing comments. 
Tests were failing in the following files-
* <code>spec/models/assignment_form_spec.rb</code>
* <code>spec/controllers/assignments_controller_spec.rb</code> 
We try to address those failing tests and add block comments before function definition wherever needed. 
Additional testing of some related files is also in the scope of the project.

=== Our Changes ===

Upon review of previous PRs, we found that the work was thorough, albeit a few test cases were failing, and comments and methods could be refactored better. Based on the prompts we got, we focussed on refactoring the create method of assignments controller, and assignments form, along with the refactoring of sidekiq-related files.

Below is the summary of all the changes as part of this PR.
==== Changes to <code>spec/controllers/auth_controller_spec.rb</code> ====
{| class="wikitable" style="width: 100%;
!  #  !! Change !! Rationale !! Commit Link
|-
|1
|Refactored the <code>create</code> method into multiple methods
| The current <code>create</code> method is long and have multiple complex references and if statements. Created separate helpers to simplify complex DB context
|[https://github.com/tusharkini/expertiza/commit/4c2f0e04e7e3ab10443af26d2868758b5c696b55 Commit]
|-
|2
|Used early return in case of errors
|We refactored the code such that the errors like the assignment already being present or the directory already being present were caught early in the code. It leads to cleaner code as well.
|[https://github.com/tusharkini/expertiza/commit/3c00648b7228eb0daf8a9afd30f8f3abfb44c2c4 Commit]
|-
|3
|Renamed variable names
|Some of the current variable names were ambiguous as to what they meant, <code>(cur_questionnaire, cur_due, etc.)</code>. Renamed those names to be less ambiguous and state their function.
|[https://github.com/tusharkini/expertiza/commit/4c2f0e04e7e3ab10443af26d2868758b5c696b55 Commit]
|-
|4
|Added comments to other helper methods
|Some functions had no comments which were needed.
|[https://github.com/tusharkini/expertiza/commit/b72febb7f2370d6ccb0c99939500f6d36178bf00 Commit]
|}

==== Changes to <code>app/mailers/mail_worker.rb</code> ====
{| class="wikitable" style="width: 100%;
!  #  !! Change !! Rationale !! Commit Link
|-
|1
|Removed inline comments and added block comments to the function perform
|Block comments before the function allows better readability and looks clean
|[https://github.com/tusharkini/expertiza/commit/b56ecca4db6e1784a3f92b4fa9619992c1d4dcb2 Commit]
|-
|2
|Moved the line to find the participants of a course inside the conditional block
|Earlier the list of participants of a course were found even when the <code>deadline_type</code> was <code>'drop_outstanding_reviews’</code>. We do find the list of emails when it is not <code>'drop_outstanding_reviews’</code> and send emails to them.
|[https://github.com/tusharkini/expertiza/commit/b56ecca4db6e1784a3f92b4fa9619992c1d4dcb2 Commit]
|-
|3
|Combined the multiple <code>attr_accessor</code>lines into a single line
|Better readability and shorter function code.
|[https://github.com/tusharkini/expertiza/commit/b56ecca4db6e1784a3f92b4fa9619992c1d4dcb2 Commit]
|-
|4
|@mail object is not used as it is not reused anywhere else
|Any temporary variable that is used once should be avoided as it takes up valuable resources.
|[https://github.com/tusharkini/expertiza/commit/b56ecca4db6e1784a3f92b4fa9619992c1d4dcb2 Commit]
|}

==== Changes to <code>spec/models/assignment_form_spec.rb</code> ====
{| class="wikitable" style="width: 100%;
!  #  !! Change !! Rationale !! Commit Link
|-
|1
| Replaced ‘mailers’ with ‘jobs’ in <code>add_to_delayed_queue</code> test
| Test was failing due to checking the wrong Sidekiq queue, currently <code>add_to_delayed_queue</code> adds to job queue NOT mailers queue
|[https://github.com/tusharkini/expertiza/commit/95dcdaaa36b837df3d580f3352b8504535cc7c2e Commit]
|}

==== Changes to <code>app/models/assignment_form.rb</code> ====
{| class="wikitable" style="width: 100%;
!  #  !! Change !! Rationale !! Commit Link
|-
|1
|Added block comments to methods and removed inline comments
|Better readability and looks clean
|[https://github.com/expertiza/expertiza/pull/2556/commits/975312c88bb65861b1b6f2b268dc8d80c15aa501 Commit]
|-
|2
|Created <code>default_assignment_questionaire</code> to handle creation of a default <code>assignment_questionnaire</code> that was originally in assignment questionnaire
|<code>assignment_questionnaire</code> originally not only searched for an <code>assignment_questionnaire</code> but created one if one matching type did not exist. Now the second functionality (creating a default <code>assignment_questionnaire</code>) is its own method.
|[https://github.com/expertiza/expertiza/pull/2556/commits/b784e829920059cbff412ad7768833e95469ff2b Commit]
|-
|3
|In line 58 of original code replaced <code>[][]</code> method of searching with <code>.dig</code>
|This particular line could throw errors and .dig is a safer way to search a table without the risk of getting nil errors
|[https://github.com/expertiza/expertiza/pull/2556/commits/975312c88bb65861b1b6f2b268dc8d80c15aa501 Commit]
|}

==== Changes to <code>spec/sidekiq_mail_worker_spec.rb</code> ====
{| class="wikitable" style="width: 100%;
!  #  !! Change !! Rationale !! Commit Link
|-
|1
|Added test case for method <code>perform</code> in <code>mail_worker.rb</code>
|This test case was not implemented in the earlier Pull Request
|[https://github.com/tusharkini/expertiza/commit/8d78d40ea4e4035f95522e025fd08e220b8b9ce4 Commit]
|-
|2
|Added test case for method <code>find_participant_emails</code> in <code>mail_worker.rb</code>
|This test case was not implemented in the earlier Pull Request
|[https://github.com/tusharkini/expertiza/commit/190ca781681021f995a8a034a34bb8cede1733e3 Commit]
|-
|3
|Added test case for method <code>email_reminder</code> in <code>mail_worker.rb</code>
|This test case was not implemented in the earlier Pull Request
|[https://github.com/tusharkini/expertiza/commit/53b8cf281215aeeec857886deaa2fae7998eff47 Commit]

|}

=== Test Coverage for some related files===
==== <code>spec/controllers/auth_controller_spec.rb</code> ====
After making the changes to assignment_form and assignment_form_spec we ran the rspec tests with the command bundle exec rspec spec/models/assignment_form_spec.rb
[[File:Assignment_form_test.jpg|1000px]]

==== <code>spec/models/assignment_form_spec.rb</code> ====
Automated testing of sidekiq_mailer_worker_spec.rb
[[File:mailer_spec.jpg|1000px]]

==== <code>spec/sidekiq_mail_worker_spec.rb</code> ====
Automated testing of assignments_controller_spec.rb
[[File:Assignment_controller_test.jpg|1000px]]

== Testing ==
VCL : http://152.7.178.223:8080/

Login: instructor_Lien

Password: password

To Test via UI

1) Login to VCL

2) Click on Manage

3) Click on Assignments

4) click on +

5) Create an assignment with a non-zero delay in SimiCheck and Similarity Threshold to a non-zero.

6) Go to the due date and add a due date for tomorrow.

7) Create the assignment

8) Go to the Assignment page and add participants (ex. Paul & Dan) to the assignment.

9) Check the sidekiq [http://152.7.178.223:8080/sidekiq] page for queued jobs.

Note: In previous refactoring changes it was noted that this error is to be expected and will not affect the real expertiza. Error is due to Simicheck Plagiarism Checker not having access to certain features in the PR build.

[[File: Functional_test_error.jpg|1000px]]

CSC/ECE 517 Spring 2023 - E2302

2023-03-23T02:47:49Z

Jtdavis9: /* spec/sidekiq_mail_worker_spec.rb */

==E2302. Refactoring the Delayed Mailer and Scheduler==

__TOC__

== Expertiza Overview ==

===Background===

Expertiza is an open source software written using Ruby on Rails which functions as a learning management software system. It has man different functions and abilities including the ability to create assignments, quizzes, assignment groups and topics, and also a complete mechanism for providing peer reviews and feedback for other groups and other teammates.

===Prior Work===

=== History: Previous projects ===

* E2253 [https://expertiza.csc.ncsu.edu/index.php/CSC/ECE_517_Fall_2022_-_E2253.Refactor_delayed_mailer.rb_and_scheduled_task.rb]
* E2144 [https://expertiza.csc.ncsu.edu/index.php/CSC/ECE_517_Fall_2021_-_E2144._Refactor_delayed_mailer_and_scheduled_task]
* E2273 [https://expertiza.csc.ncsu.edu/index.php/CSC/ECE_517_Fall_2022_-_E2273.Refactor_delayed_mailer.rb_and_scheduled_task.rb]

The latest Pull Request was not merged into the repository due to some failing tests and missing comments. 
Tests were failing in the following files-
* <code>spec/models/assignment_form_spec.rb</code>
* <code>spec/controllers/assignments_controller_spec.rb</code> 
We try to address those failing tests and add block comments before function definition wherever needed. 
Additional testing of some related files is also in the scope of the project.

=== Our Changes ===

Upon review of previous PRs, we found that the work was thorough, albeit a few test cases were failing, and comments and methods could be refactored better. Based on the prompts we got, we focussed on refactoring the create method of assignments controller, and assignments form, along with the refactoring of sidekiq-related files.

Below is the summary of all the changes as part of this PR.
==== Changes to <code>spec/controllers/auth_controller_spec.rb</code> ====
{| class="wikitable" style="width: 100%;
!  #  !! Change !! Rationale !! Commit Link
|-
|1
|Refactored the <code>create</code> method into multiple methods
| The current <code>create</code> method is long and have multiple complex references and if statements. Created separate helpers to simplify complex DB context
|[https://github.com/tusharkini/expertiza/commit/4c2f0e04e7e3ab10443af26d2868758b5c696b55 Commit]
|-
|2
|Used early return in case of errors
|We refactored the code such that the errors like the assignment already being present or the directory already being present were caught early in the code. It leads to cleaner code as well.
|[https://github.com/tusharkini/expertiza/commit/3c00648b7228eb0daf8a9afd30f8f3abfb44c2c4 Commit]
|-
|3
|Renamed variable names
|Some of the current variable names were ambiguous as to what they meant, <code>(cur_questionnaire, cur_due, etc.)</code>. Renamed those names to be less ambiguous and state their function.
|[https://github.com/tusharkini/expertiza/commit/4c2f0e04e7e3ab10443af26d2868758b5c696b55 Commit]
|-
|4
|Added comments to other helper methods
|Some functions had no comments which were needed.
|[https://github.com/tusharkini/expertiza/commit/b72febb7f2370d6ccb0c99939500f6d36178bf00 Commit]
|}

==== Changes to <code>app/mailers/mail_worker.rb</code> ====
{| class="wikitable" style="width: 100%;
!  #  !! Change !! Rationale !! Commit Link
|-
|1
|Removed inline comments and added block comments to the function perform
|Block comments before the function allows better readability and looks clean
|[https://github.com/tusharkini/expertiza/commit/b56ecca4db6e1784a3f92b4fa9619992c1d4dcb2 Commit]
|-
|2
|Moved the line to find the participants of a course inside the conditional block
|Earlier the list of participants of a course were found even when the <code>deadline_type</code> was <code>'drop_outstanding_reviews’</code>. We do find the list of emails when it is not <code>'drop_outstanding_reviews’</code> and send emails to them.
|[https://github.com/tusharkini/expertiza/commit/b56ecca4db6e1784a3f92b4fa9619992c1d4dcb2 Commit]
|-
|3
|Combined the multiple <code>attr_accessor</code>lines into a single line
|Better readability and shorter function code.
|[https://github.com/tusharkini/expertiza/commit/b56ecca4db6e1784a3f92b4fa9619992c1d4dcb2 Commit]
|-
|4
|@mail object is not used as it is not reused anywhere else
|Any temporary variable that is used once should be avoided as it takes up valuable resources.
|[https://github.com/tusharkini/expertiza/commit/b56ecca4db6e1784a3f92b4fa9619992c1d4dcb2 Commit]
|}

==== Changes to <code>spec/models/assignment_form_spec.rb</code> ====
{| class="wikitable" style="width: 100%;
!  #  !! Change !! Rationale !! Commit Link
|-
|1
| Replaced ‘mailers’ with ‘jobs’ in <code>add_to_delayed_queue</code> test
| Test was failing due to checking the wrong Sidekiq queue, currently <code>add_to_delayed_queue</code> adds to job queue NOT mailers queue
|[https://github.com/tusharkini/expertiza/commit/95dcdaaa36b837df3d580f3352b8504535cc7c2e Commit]
|}

==== Changes to <code>app/models/assignment_form.rb</code> ====
{| class="wikitable" style="width: 100%;
!  #  !! Change !! Rationale !! Commit Link
|-
|1
|Added block comments to methods and removed inline comments
|Better readability and looks clean
|[https://github.com/expertiza/expertiza/pull/2556/commits/975312c88bb65861b1b6f2b268dc8d80c15aa501 Commit]
|-
|2
|Created <code>default_assignment_questionaire</code> to handle creation of a default <code>assignment_questionnaire</code> that was originally in assignment questionnaire
|<code>assignment_questionnaire</code> originally not only searched for an <code>assignment_questionnaire</code> but created one if one matching type did not exist. Now the second functionality (creating a default <code>assignment_questionnaire</code>) is its own method.
|[https://github.com/expertiza/expertiza/pull/2556/commits/b784e829920059cbff412ad7768833e95469ff2b Commit]
|-
|3
|In line 58 of original code replaced <code>[][]</code> method of searching with <code>.dig</code>
|This particular line could throw errors and .dig is a safer way to search a table without the risk of getting nil errors
|[https://github.com/expertiza/expertiza/pull/2556/commits/975312c88bb65861b1b6f2b268dc8d80c15aa501 Commit]
|}

==== Changes to <code>spec/sidekiq_mail_worker_spec.rb</code> ====
{| class="wikitable" style="width: 100%;
!  #  !! Change !! Rationale !! Commit Link
|-
|1
|Added test case for method <code>perform</code> in <code>mail_worker.rb</code>
|This test case was not implemented in the earlier Pull Request
|[https://github.com/tusharkini/expertiza/commit/8d78d40ea4e4035f95522e025fd08e220b8b9ce4 Commit]
|-
|2
|Added test case for method <code>find_participant_emails</code> in <code>mail_worker.rb</code>
|This test case was not implemented in the earlier Pull Request
|[https://github.com/tusharkini/expertiza/commit/190ca781681021f995a8a034a34bb8cede1733e3 Commit]
|-
|3
|Added test case for method <code>email_reminder</code> in <code>mail_worker.rb</code>
|This test case was not implemented in the earlier Pull Request
|[https://github.com/tusharkini/expertiza/commit/53b8cf281215aeeec857886deaa2fae7998eff47 Commit]

|}

=== Test Coverage for some related files===
==== <code>spec/controllers/auth_controller_spec.rb</code> ====
After making the changes to assignment_form and assignment_form_spec we ran the rspec tests with the command bundle exec rspec spec/models/assignment_form_spec.rb
[[File:Assignment_form_test.jpg|1000px]]

==== <code>spec/models/assignment_form_spec.rb</code> ====
Automated testing of sidekiq_mailer_worker_spec.rb
[[File:mailer_spec.jpg|1000px]]

==== <code>spec/sidekiq_mail_worker_spec.rb</code> ====
Automated testing of assignments_controller_spec.rb
[[File:Assignment_controller_test.jpg|1000px]]

== Testing ==
VCL : http://152.7.178.223:8080/

Login: instructor_Lien
Password: Password

To Test via UI

1) Login to VCL

2) Click on Manage

3) Click on Assignments

4) click on +

5) Create an assignment with a non-zero delay in SimiCheck and Similarity Threshold to a non-zero.

6) Go to the due date and add a due date for tomorrow.

7) Create the assignment

8) Go to the Assignment page and add participants to the assignment.

9) Check the sidekiq page for queued jobs.

CSC/ECE 517 Spring 2023 - E2302

2023-03-23T02:43:27Z

Jtdavis9: /* spec/models/assignment_form_spec.rb */

==E2302. Refactoring the Delayed Mailer and Scheduler==

__TOC__

== Expertiza Overview ==

===Background===

Expertiza is an open source software written using Ruby on Rails which functions as a learning management software system. It has man different functions and abilities including the ability to create assignments, quizzes, assignment groups and topics, and also a complete mechanism for providing peer reviews and feedback for other groups and other teammates.

===Prior Work===

=== History: Previous projects ===

* E2253 [https://expertiza.csc.ncsu.edu/index.php/CSC/ECE_517_Fall_2022_-_E2253.Refactor_delayed_mailer.rb_and_scheduled_task.rb]
* E2144 [https://expertiza.csc.ncsu.edu/index.php/CSC/ECE_517_Fall_2021_-_E2144._Refactor_delayed_mailer_and_scheduled_task]
* E2273 [https://expertiza.csc.ncsu.edu/index.php/CSC/ECE_517_Fall_2022_-_E2273.Refactor_delayed_mailer.rb_and_scheduled_task.rb]

The latest Pull Request was not merged into the repository due to some failing tests and missing comments. 
Tests were failing in the following files-
* <code>spec/models/assignment_form_spec.rb</code>
* <code>spec/controllers/assignments_controller_spec.rb</code> 
We try to address those failing tests and add block comments before function definition wherever needed. 
Additional testing of some related files is also in the scope of the project.

=== Our Changes ===

Upon review of previous PRs, we found that the work was thorough, albeit a few test cases were failing, and comments and methods could be refactored better. Based on the prompts we got, we focussed on refactoring the create method of assignments controller, and assignments form, along with the refactoring of sidekiq-related files.

Below is the summary of all the changes as part of this PR.
==== Changes to <code>spec/controllers/auth_controller_spec.rb</code> ====
{| class="wikitable" style="width: 100%;
!  #  !! Change !! Rationale !! Commit Link
|-
|1
|Refactored the <code>create</code> method into multiple methods
| The current <code>create</code> method is long and have multiple complex references and if statements. Created separate helpers to simplify complex DB context
|[https://github.com/tusharkini/expertiza/commit/4c2f0e04e7e3ab10443af26d2868758b5c696b55 Commit]
|-
|2
|Used early return in case of errors
|We refactored the code such that the errors like the assignment already being present or the directory already being present were caught early in the code. It leads to cleaner code as well.
|[https://github.com/tusharkini/expertiza/commit/3c00648b7228eb0daf8a9afd30f8f3abfb44c2c4 Commit]
|-
|3
|Renamed variable names
|Some of the current variable names were ambiguous as to what they meant, <code>(cur_questionnaire, cur_due, etc.)</code>. Renamed those names to be less ambiguous and state their function.
|[https://github.com/tusharkini/expertiza/commit/4c2f0e04e7e3ab10443af26d2868758b5c696b55 Commit]
|-
|4
|Added comments to other helper methods
|Some functions had no comments which were needed.
|[https://github.com/tusharkini/expertiza/commit/b72febb7f2370d6ccb0c99939500f6d36178bf00 Commit]
|}

==== Changes to <code>app/mailers/mail_worker.rb</code> ====
{| class="wikitable" style="width: 100%;
!  #  !! Change !! Rationale !! Commit Link
|-
|1
|Removed inline comments and added block comments to the function perform
|Block comments before the function allows better readability and looks clean
|[https://github.com/tusharkini/expertiza/commit/b56ecca4db6e1784a3f92b4fa9619992c1d4dcb2 Commit]
|-
|2
|Moved the line to find the participants of a course inside the conditional block
|Earlier the list of participants of a course were found even when the <code>deadline_type</code> was <code>'drop_outstanding_reviews’</code>. We do find the list of emails when it is not <code>'drop_outstanding_reviews’</code> and send emails to them.
|[https://github.com/tusharkini/expertiza/commit/b56ecca4db6e1784a3f92b4fa9619992c1d4dcb2 Commit]
|-
|3
|Combined the multiple <code>attr_accessor</code>lines into a single line
|Better readability and shorter function code.
|[https://github.com/tusharkini/expertiza/commit/b56ecca4db6e1784a3f92b4fa9619992c1d4dcb2 Commit]
|-
|4
|@mail object is not used as it is not reused anywhere else
|Any temporary variable that is used once should be avoided as it takes up valuable resources.
|[https://github.com/tusharkini/expertiza/commit/b56ecca4db6e1784a3f92b4fa9619992c1d4dcb2 Commit]
|}

==== Changes to <code>spec/models/assignment_form_spec.rb</code> ====
{| class="wikitable" style="width: 100%;
!  #  !! Change !! Rationale !! Commit Link
|-
|1
| Replaced ‘mailers’ with ‘jobs’ in <code>add_to_delayed_queue</code> test
| Test was failing due to checking the wrong Sidekiq queue, currently <code>add_to_delayed_queue</code> adds to job queue NOT mailers queue
|[https://github.com/tusharkini/expertiza/commit/95dcdaaa36b837df3d580f3352b8504535cc7c2e Commit]
|}

==== Changes to <code>app/models/assignment_form.rb</code> ====
{| class="wikitable" style="width: 100%;
!  #  !! Change !! Rationale !! Commit Link
|-
|1
|Added block comments to methods and removed inline comments
|Better readability and looks clean
|[https://github.com/expertiza/expertiza/pull/2556/commits/975312c88bb65861b1b6f2b268dc8d80c15aa501 Commit]
|-
|2
|Created <code>default_assignment_questionaire</code> to handle creation of a default <code>assignment_questionnaire</code> that was originally in assignment questionnaire
|<code>assignment_questionnaire</code> originally not only searched for an <code>assignment_questionnaire</code> but created one if one matching type did not exist. Now the second functionality (creating a default <code>assignment_questionnaire</code>) is its own method.
|[https://github.com/expertiza/expertiza/pull/2556/commits/b784e829920059cbff412ad7768833e95469ff2b Commit]
|-
|3
|In line 58 of original code replaced <code>[][]</code> method of searching with <code>.dig</code>
|This particular line could throw errors and .dig is a safer way to search a table without the risk of getting nil errors
|[https://github.com/expertiza/expertiza/pull/2556/commits/975312c88bb65861b1b6f2b268dc8d80c15aa501 Commit]
|}

==== Changes to <code>spec/sidekiq_mail_worker_spec.rb</code> ====
{| class="wikitable" style="width: 100%;
!  #  !! Change !! Rationale !! Commit Link
|-
|1
|Added test case for method <code>perform</code> in <code>mail_worker.rb</code>
|This test case was not implemented in the earlier Pull Request
|[https://github.com/tusharkini/expertiza/commit/8d78d40ea4e4035f95522e025fd08e220b8b9ce4 Commit]
|-
|2
|Added test case for method <code>find_participant_emails</code> in <code>mail_worker.rb</code>
|This test case was not implemented in the earlier Pull Request
|[https://github.com/tusharkini/expertiza/commit/190ca781681021f995a8a034a34bb8cede1733e3 Commit]
|-
|3
|Added test case for method <code>email_reminder</code> in <code>mail_worker.rb</code>
|This test case was not implemented in the earlier Pull Request
|[https://github.com/tusharkini/expertiza/commit/53b8cf281215aeeec857886deaa2fae7998eff47 Commit]

|}

=== Test Coverage for some related files===
==== <code>spec/controllers/auth_controller_spec.rb</code> ====
After making the changes to assignment_form and assignment_form_spec we ran the rspec tests with the command bundle exec rspec spec/models/assignment_form_spec.rb
[[File:Assignment_form_test.jpg|1000px]]

==== <code>spec/models/assignment_form_spec.rb</code> ====
Automated testing of sidekiq_mailer_worker_spec.rb
[[File:mailer_spec.jpg|1000px]]

==== <code>spec/sidekiq_mail_worker_spec.rb</code> ====

== Testing ==
VCL : http://152.7.177.156:8080/
Login: instructor_Lien
Password: Password

To Test via UI

1) Login to VCL

CSC/ECE 517 Spring 2023 - E2302

2023-03-23T02:41:26Z

Jtdavis9: /* spec/controllers/auth_controller_spec.rb */

==E2302. Refactoring the Delayed Mailer and Scheduler==

__TOC__

== Expertiza Overview ==

===Background===

Expertiza is an open source software written using Ruby on Rails which functions as a learning management software system. It has man different functions and abilities including the ability to create assignments, quizzes, assignment groups and topics, and also a complete mechanism for providing peer reviews and feedback for other groups and other teammates.

===Prior Work===

=== History: Previous projects ===

* E2253 [https://expertiza.csc.ncsu.edu/index.php/CSC/ECE_517_Fall_2022_-_E2253.Refactor_delayed_mailer.rb_and_scheduled_task.rb]
* E2144 [https://expertiza.csc.ncsu.edu/index.php/CSC/ECE_517_Fall_2021_-_E2144._Refactor_delayed_mailer_and_scheduled_task]
* E2273 [https://expertiza.csc.ncsu.edu/index.php/CSC/ECE_517_Fall_2022_-_E2273.Refactor_delayed_mailer.rb_and_scheduled_task.rb]

The latest Pull Request was not merged into the repository due to some failing tests and missing comments. 
Tests were failing in the following files-
* <code>spec/models/assignment_form_spec.rb</code>
* <code>spec/controllers/assignments_controller_spec.rb</code> 
We try to address those failing tests and add block comments before function definition wherever needed. 
Additional testing of some related files is also in the scope of the project.

=== Our Changes ===

Upon review of previous PRs, we found that the work was thorough, albeit a few test cases were failing, and comments and methods could be refactored better. Based on the prompts we got, we focussed on refactoring the create method of assignments controller, and assignments form, along with the refactoring of sidekiq-related files.

Below is the summary of all the changes as part of this PR.
==== Changes to <code>spec/controllers/auth_controller_spec.rb</code> ====
{| class="wikitable" style="width: 100%;
!  #  !! Change !! Rationale !! Commit Link
|-
|1
|Refactored the <code>create</code> method into multiple methods
| The current <code>create</code> method is long and have multiple complex references and if statements. Created separate helpers to simplify complex DB context
|[https://github.com/tusharkini/expertiza/commit/4c2f0e04e7e3ab10443af26d2868758b5c696b55 Commit]
|-
|2
|Used early return in case of errors
|We refactored the code such that the errors like the assignment already being present or the directory already being present were caught early in the code. It leads to cleaner code as well.
|[https://github.com/tusharkini/expertiza/commit/3c00648b7228eb0daf8a9afd30f8f3abfb44c2c4 Commit]
|-
|3
|Renamed variable names
|Some of the current variable names were ambiguous as to what they meant, <code>(cur_questionnaire, cur_due, etc.)</code>. Renamed those names to be less ambiguous and state their function.
|[https://github.com/tusharkini/expertiza/commit/4c2f0e04e7e3ab10443af26d2868758b5c696b55 Commit]
|-
|4
|Added comments to other helper methods
|Some functions had no comments which were needed.
|[https://github.com/tusharkini/expertiza/commit/b72febb7f2370d6ccb0c99939500f6d36178bf00 Commit]
|}

==== Changes to <code>app/mailers/mail_worker.rb</code> ====
{| class="wikitable" style="width: 100%;
!  #  !! Change !! Rationale !! Commit Link
|-
|1
|Removed inline comments and added block comments to the function perform
|Block comments before the function allows better readability and looks clean
|[https://github.com/tusharkini/expertiza/commit/b56ecca4db6e1784a3f92b4fa9619992c1d4dcb2 Commit]
|-
|2
|Moved the line to find the participants of a course inside the conditional block
|Earlier the list of participants of a course were found even when the <code>deadline_type</code> was <code>'drop_outstanding_reviews’</code>. We do find the list of emails when it is not <code>'drop_outstanding_reviews’</code> and send emails to them.
|[https://github.com/tusharkini/expertiza/commit/b56ecca4db6e1784a3f92b4fa9619992c1d4dcb2 Commit]
|-
|3
|Combined the multiple <code>attr_accessor</code>lines into a single line
|Better readability and shorter function code.
|[https://github.com/tusharkini/expertiza/commit/b56ecca4db6e1784a3f92b4fa9619992c1d4dcb2 Commit]
|-
|4
|@mail object is not used as it is not reused anywhere else
|Any temporary variable that is used once should be avoided as it takes up valuable resources.
|[https://github.com/tusharkini/expertiza/commit/b56ecca4db6e1784a3f92b4fa9619992c1d4dcb2 Commit]
|}

==== Changes to <code>spec/models/assignment_form_spec.rb</code> ====
{| class="wikitable" style="width: 100%;
!  #  !! Change !! Rationale !! Commit Link
|-
|1
| Replaced ‘mailers’ with ‘jobs’ in <code>add_to_delayed_queue</code> test
| Test was failing due to checking the wrong Sidekiq queue, currently <code>add_to_delayed_queue</code> adds to job queue NOT mailers queue
|[https://github.com/tusharkini/expertiza/commit/95dcdaaa36b837df3d580f3352b8504535cc7c2e Commit]
|}

==== Changes to <code>app/models/assignment_form.rb</code> ====
{| class="wikitable" style="width: 100%;
!  #  !! Change !! Rationale !! Commit Link
|-
|1
|Added block comments to methods and removed inline comments
|Better readability and looks clean
|[https://github.com/expertiza/expertiza/pull/2556/commits/975312c88bb65861b1b6f2b268dc8d80c15aa501 Commit]
|-
|2
|Created <code>default_assignment_questionaire</code> to handle creation of a default <code>assignment_questionnaire</code> that was originally in assignment questionnaire
|<code>assignment_questionnaire</code> originally not only searched for an <code>assignment_questionnaire</code> but created one if one matching type did not exist. Now the second functionality (creating a default <code>assignment_questionnaire</code>) is its own method.
|[https://github.com/expertiza/expertiza/pull/2556/commits/b784e829920059cbff412ad7768833e95469ff2b Commit]
|-
|3
|In line 58 of original code replaced <code>[][]</code> method of searching with <code>.dig</code>
|This particular line could throw errors and .dig is a safer way to search a table without the risk of getting nil errors
|[https://github.com/expertiza/expertiza/pull/2556/commits/975312c88bb65861b1b6f2b268dc8d80c15aa501 Commit]
|}

==== Changes to <code>spec/sidekiq_mail_worker_spec.rb</code> ====
{| class="wikitable" style="width: 100%;
!  #  !! Change !! Rationale !! Commit Link
|-
|1
|Added test case for method <code>perform</code> in <code>mail_worker.rb</code>
|This test case was not implemented in the earlier Pull Request
|[https://github.com/tusharkini/expertiza/commit/8d78d40ea4e4035f95522e025fd08e220b8b9ce4 Commit]
|-
|2
|Added test case for method <code>find_participant_emails</code> in <code>mail_worker.rb</code>
|This test case was not implemented in the earlier Pull Request
|[https://github.com/tusharkini/expertiza/commit/190ca781681021f995a8a034a34bb8cede1733e3 Commit]
|-
|3
|Added test case for method <code>email_reminder</code> in <code>mail_worker.rb</code>
|This test case was not implemented in the earlier Pull Request
|[https://github.com/tusharkini/expertiza/commit/53b8cf281215aeeec857886deaa2fae7998eff47 Commit]

|}

=== Test Coverage for some related files===
==== <code>spec/controllers/auth_controller_spec.rb</code> ====
After making the changes to assignment_form and assignment_form_spec we ran the rspec tests with the command bundle exec rspec spec/models/assignment_form_spec.rb
[[File:Assignment_form_test.jpg|1000px]]

==== <code>spec/models/assignment_form_spec.rb</code> ====

==== <code>spec/sidekiq_mail_worker_spec.rb</code> ====

== Testing ==
VCL : http://152.7.177.156:8080/
Login: instructor_Lien
Password: Password

To Test via UI

1) Login to VCL

CSC/ECE 517 Spring 2023 - E2302

2023-03-23T02:38:32Z

Jtdavis9: /* spec/controllers/auth_controller_spec.rb */

==E2302. Refactoring the Delayed Mailer and Scheduler==

__TOC__

== Expertiza Overview ==

===Background===

Expertiza is an open source software written using Ruby on Rails which functions as a learning management software system. It has man different functions and abilities including the ability to create assignments, quizzes, assignment groups and topics, and also a complete mechanism for providing peer reviews and feedback for other groups and other teammates.

===Prior Work===

=== History: Previous projects ===

* E2253 [https://expertiza.csc.ncsu.edu/index.php/CSC/ECE_517_Fall_2022_-_E2253.Refactor_delayed_mailer.rb_and_scheduled_task.rb]
* E2144 [https://expertiza.csc.ncsu.edu/index.php/CSC/ECE_517_Fall_2021_-_E2144._Refactor_delayed_mailer_and_scheduled_task]
* E2273 [https://expertiza.csc.ncsu.edu/index.php/CSC/ECE_517_Fall_2022_-_E2273.Refactor_delayed_mailer.rb_and_scheduled_task.rb]

The latest Pull Request was not merged into the repository due to some failing tests and missing comments. 
Tests were failing in the following files-
* <code>spec/models/assignment_form_spec.rb</code>
* <code>spec/controllers/assignments_controller_spec.rb</code> 
We try to address those failing tests and add block comments before function definition wherever needed. 
Additional testing of some related files is also in the scope of the project.

=== Our Changes ===

Upon review of previous PRs, we found that the work was thorough, albeit a few test cases were failing, and comments and methods could be refactored better. Based on the prompts we got, we focussed on refactoring the create method of assignments controller, and assignments form, along with the refactoring of sidekiq-related files.

Below is the summary of all the changes as part of this PR.
==== Changes to <code>spec/controllers/auth_controller_spec.rb</code> ====
{| class="wikitable" style="width: 100%;
!  #  !! Change !! Rationale !! Commit Link
|-
|1
|Refactored the <code>create</code> method into multiple methods
| The current <code>create</code> method is long and have multiple complex references and if statements. Created separate helpers to simplify complex DB context
|[https://github.com/tusharkini/expertiza/commit/4c2f0e04e7e3ab10443af26d2868758b5c696b55 Commit]
|-
|2
|Used early return in case of errors
|We refactored the code such that the errors like the assignment already being present or the directory already being present were caught early in the code. It leads to cleaner code as well.
|[https://github.com/tusharkini/expertiza/commit/3c00648b7228eb0daf8a9afd30f8f3abfb44c2c4 Commit]
|-
|3
|Renamed variable names
|Some of the current variable names were ambiguous as to what they meant, <code>(cur_questionnaire, cur_due, etc.)</code>. Renamed those names to be less ambiguous and state their function.
|[https://github.com/tusharkini/expertiza/commit/4c2f0e04e7e3ab10443af26d2868758b5c696b55 Commit]
|-
|4
|Added comments to other helper methods
|Some functions had no comments which were needed.
|[https://github.com/tusharkini/expertiza/commit/b72febb7f2370d6ccb0c99939500f6d36178bf00 Commit]
|}

==== Changes to <code>app/mailers/mail_worker.rb</code> ====
{| class="wikitable" style="width: 100%;
!  #  !! Change !! Rationale !! Commit Link
|-
|1
|Removed inline comments and added block comments to the function perform
|Block comments before the function allows better readability and looks clean
|[https://github.com/tusharkini/expertiza/commit/b56ecca4db6e1784a3f92b4fa9619992c1d4dcb2 Commit]
|-
|2
|Moved the line to find the participants of a course inside the conditional block
|Earlier the list of participants of a course were found even when the <code>deadline_type</code> was <code>'drop_outstanding_reviews’</code>. We do find the list of emails when it is not <code>'drop_outstanding_reviews’</code> and send emails to them.
|[https://github.com/tusharkini/expertiza/commit/b56ecca4db6e1784a3f92b4fa9619992c1d4dcb2 Commit]
|-
|3
|Combined the multiple <code>attr_accessor</code>lines into a single line
|Better readability and shorter function code.
|[https://github.com/tusharkini/expertiza/commit/b56ecca4db6e1784a3f92b4fa9619992c1d4dcb2 Commit]
|-
|4
|@mail object is not used as it is not reused anywhere else
|Any temporary variable that is used once should be avoided as it takes up valuable resources.
|[https://github.com/tusharkini/expertiza/commit/b56ecca4db6e1784a3f92b4fa9619992c1d4dcb2 Commit]
|}

==== Changes to <code>spec/models/assignment_form_spec.rb</code> ====
{| class="wikitable" style="width: 100%;
!  #  !! Change !! Rationale !! Commit Link
|-
|1
| Replaced ‘mailers’ with ‘jobs’ in <code>add_to_delayed_queue</code> test
| Test was failing due to checking the wrong Sidekiq queue, currently <code>add_to_delayed_queue</code> adds to job queue NOT mailers queue
|[https://github.com/tusharkini/expertiza/commit/95dcdaaa36b837df3d580f3352b8504535cc7c2e Commit]
|}

==== Changes to <code>app/models/assignment_form.rb</code> ====
{| class="wikitable" style="width: 100%;
!  #  !! Change !! Rationale !! Commit Link
|-
|1
|Added block comments to methods and removed inline comments
|Better readability and looks clean
|[https://github.com/expertiza/expertiza/pull/2556/commits/975312c88bb65861b1b6f2b268dc8d80c15aa501 Commit]
|-
|2
|Created <code>default_assignment_questionaire</code> to handle creation of a default <code>assignment_questionnaire</code> that was originally in assignment questionnaire
|<code>assignment_questionnaire</code> originally not only searched for an <code>assignment_questionnaire</code> but created one if one matching type did not exist. Now the second functionality (creating a default <code>assignment_questionnaire</code>) is its own method.
|[https://github.com/expertiza/expertiza/pull/2556/commits/b784e829920059cbff412ad7768833e95469ff2b Commit]
|-
|3
|In line 58 of original code replaced <code>[][]</code> method of searching with <code>.dig</code>
|This particular line could throw errors and .dig is a safer way to search a table without the risk of getting nil errors
|[https://github.com/expertiza/expertiza/pull/2556/commits/975312c88bb65861b1b6f2b268dc8d80c15aa501 Commit]
|}

==== Changes to <code>spec/sidekiq_mail_worker_spec.rb</code> ====
{| class="wikitable" style="width: 100%;
!  #  !! Change !! Rationale !! Commit Link
|-
|1
|Added test case for method <code>perform</code> in <code>mail_worker.rb</code>
|This test case was not implemented in the earlier Pull Request
|[https://github.com/tusharkini/expertiza/commit/8d78d40ea4e4035f95522e025fd08e220b8b9ce4 Commit]
|-
|2
|Added test case for method <code>find_participant_emails</code> in <code>mail_worker.rb</code>
|This test case was not implemented in the earlier Pull Request
|[https://github.com/tusharkini/expertiza/commit/190ca781681021f995a8a034a34bb8cede1733e3 Commit]
|-
|3
|Added test case for method <code>email_reminder</code> in <code>mail_worker.rb</code>
|This test case was not implemented in the earlier Pull Request
|[https://github.com/tusharkini/expertiza/commit/53b8cf281215aeeec857886deaa2fae7998eff47 Commit]

|}

=== Test Coverage for some related files===
==== <code>spec/controllers/auth_controller_spec.rb</code> ====
[[File:Assignment_form_test.jpg|1000px]]

==== <code>spec/models/assignment_form_spec.rb</code> ====

==== <code>spec/sidekiq_mail_worker_spec.rb</code> ====

File:Assignment form test.jpg

2023-03-23T02:35:58Z

Jtdavis9:

File:Mailer spec.jpg

2023-03-23T02:31:47Z

Jtdavis9:

File:Functional test error.jpg

2023-03-23T02:30:45Z

Jtdavis9:

File:Assignment controller test.jpg

2023-03-23T02:30:34Z

Jtdavis9: Jtdavis9 uploaded a new version of File:Assignment controller test.jpg

File:Assignment controller test.jpg

2023-03-23T02:29:59Z

Jtdavis9:

CSC/ECE 517 Spring 2023 - E2302

2023-03-22T15:09:19Z

Jtdavis9:

==E2302. Refactoring the Delayed Mailer and Scheduler==

__TOC__

== Topic Overview ==

===Prior Work===

=== History: Previous projects ===

* E2253 [https://expertiza.csc.ncsu.edu/index.php/CSC/ECE_517_Fall_2022_-_E2253.Refactor_delayed_mailer.rb_and_scheduled_task.rb]
* E2144 [https://expertiza.csc.ncsu.edu/index.php/CSC/ECE_517_Fall_2021_-_E2144._Refactor_delayed_mailer_and_scheduled_task]
* E2273 [https://expertiza.csc.ncsu.edu/index.php/CSC/ECE_517_Fall_2022_-_E2273.Refactor_delayed_mailer.rb_and_scheduled_task.rb]

== Our Changes ==

CSC/ECE 517 Spring 2023 - E2302

2023-03-20T18:29:28Z

Jtdavis9: Created page with "==E2302. Refactoring the Delayed Mailer and Scheduler== __TOC__"

==E2302. Refactoring the Delayed Mailer and Scheduler==

__TOC__