CSC/ECE 517 Spring 2022 - E2218: Refactor response controller: Difference between revisions

From Expertiza_Wiki
Jump to navigation Jump to search
No edit summary
 
(15 intermediate revisions by 2 users not shown)
Line 4: Line 4:
== Background ==
== Background ==


A response is the object that is created when someone fills out a review rubric, such as when one writes a review, gives feedback to a reviewer, or fills out a survey.  Responses to the individual rubric items are kept in Answer objects; each Answer object has a response_id to say what Response it is part of.  Since response_controller needs to work with many kinds of responses, its code is pretty general.  It is not the worst controller in the system, but it would be much clearer if its method names were more descriptive of what they do.
A response is an object that is created when someone fills out a review rubric, such as when one writes a review, gives feedback to a reviewer, or fills out a survey.  Responses to the individual rubric items are kept in Answer objects; each Answer object has a response_id to say what Response it is part of.  Since response_controller needs to work with many kinds of responses, its code is pretty general.  It is not the worst controller in the system, but it would be much clearer if its method names were more descriptive of what they do.


=== Mentor ===
=== Mentor ===
Line 15: Line 15:
* Akhil Kumar Mengani (amengan@ncsu.edu)
* Akhil Kumar Mengani (amengan@ncsu.edu)
* Samson Reddy Mulkur (smulkur@ncsu.edu)
* Samson Reddy Mulkur (smulkur@ncsu.edu)
=== Problem Statement ===
1) After providing a review of a particular project, calibration results are useful to differentiate the review given by the reviewer and that of the instructor's review for the same project. The main problem solved is to refactor the ''show_calibration_results_for_student'' method in the response controller. This method is called when the user clicks on show calibration results as shown in the UI Testing section.
Inside the ''show_calibration_results_for_student'' method, a call to the ''calibration_results_info'' method is made which is part of the response model. This particular method is returning three values calibration_response, review_response, questions. In general, it is not a good approach to return the array of three values from the model method as the controller method has no idea what would be the return value from the model method. We have done refactoring for this which is explained in the Modified files and Refactorization section.
2) In the ''Save'' method inside the response controller, changes are made which are specific to awarded badge model. But those changes should be part of a method present inside the awarded badge model and the controller should call that method. Changes are made for the same which are described in the below section.
3) There are code climate issues that are being fixed and are described in detail in the below section.


== Modified Files and Refactorization ==
== Modified Files and Refactorization ==
A. '''response_controller.rb''' <br>
A. '''response_controller.rb''' <br>
1.  '''authorize_show_calibration_results'''
 
      This method is used to authorize if the user(reviewer) is allowed to view the calibration results for the corresponding ''calibration_response_map_id'' and ''review_response_map_id''  
  '''authorize_show_calibration_results'''
      passed as parameters in the URL.
  This method is used to authorize if the user(reviewer) is allowed to view the calibration results for the corresponding  
      Ex: https://expertiza.ncsu.edu/response/show_calibration_results_for_student?calibration_response_map_id=165688&review_response_map_id=165790
  ''calibration_response_map_id'' and ''review_response_map_id''  
      When someone tries to change the map_id's, the request should be authorized to check if the user can access the calibration result or not. <br>
  passed as parameters in the URL.
  Ex: https://expertiza.ncsu.edu/response/show_calibration_results_for_student?calibration_response_map_id=165688&review_response_map_id=165790
  When someone tries to change the map_id's, the request should be authorized to check if the user can access the calibration result or not. <br>
  [[File:Authorize_show_calibration_results.png|1180px|]]
 
  '''set_response'''
  @response and @map instance variables are initialized the same way across multiple methods.
  In order to avoid duplication, we created a before_action action which will be called first before calling the corresponding methods.
  [[File:Set_response_method.png|1170px|]]


      [[File:Authorize_show_calibration_results.png|1200px|]]


B. '''response_helper.rb''' <br>
B. '''response_helper.rb''' <br>
1 . Following methods are moved from the ''response_controller.rb'' class to ''response_helper.rb'' class since these methods are called from the controller action methods.
 
  Following methods are moved from the ''response_controller.rb'' class to ''response_helper.rb'' class since these methods are called from the  
  controller action methods.
     i) '''current_user_is_reviewer'''  
     i) '''current_user_is_reviewer'''  
     ii) '''sort_questions'''
     ii) '''sort_questions'''
Line 33: Line 52:
     iv) '''set_content''' <br>
     iv) '''set_content''' <br>
      
      
     [[File:Response_helper_1.png|1200px|]]
     [[File:Response_helper_1.png|1170px|]]


     [[File:Response_helper_2.png|1200px|]]
     [[File:Response_helper_2.png|1170px|]]


C. '''assignment_questionnaire.rb''' <br>
C. '''assignment_questionnaire.rb''' <br>
1. ''get_questions_by_assignment_id''
  '''get_questions_by_assignment_id'''
      Parameters : assignment_id  
  Parameters : assignment_id  
      When a user clicks on 'calibration results' from UI, questions in the assignment that has been reviewed are displayed.
  When a user clicks on 'calibration results' from UI, questions in the assignment that has been reviewed are displayed.
      A new method is added in the ''AssignmentQuestionnaire'' model class that returns the appropriate questions by querying the ''AssignmentQuestionnaire'' using the ''assignment_id''.
  A new method is added in the ''AssignmentQuestionnaire'' model class that returns the appropriate questions by querying the  
      This method is added to separate the business logic of retrieving the questions from the controller class.
  ''AssignmentQuestionnaire'' using the ''assignment_id''.
  This method is added to separate the business logic of retrieving the questions from the controller class.
      
      
    [[File: Get_questions_by_assignment_id.png|1200px|]]
  [[File: Get_questions_by_assignment_id.png|1170px|]]


D. '''awarded_badge.rb'''  <br>
D. '''awarded_badge.rb'''  <br>
1. ''award_badge''
  '''award_badge'''
    Parameters :  participant_id, badge_name
  Parameters :  participant_id, badge_name
    This method adds the ''badge_name'' to a specific ''participant_id'' in the AwardedBadges table.  
  This method adds the ''badge_name'' to a specific ''participant_id'' in the AwardedBadges table.  
    A separate method is created in the ''AwardedBadges'' model class to avoid the controller class from knowing the business logic of adding a badge name to a participant.
  A separate method is created in the ''AwardedBadges'' model class to avoid the controller class from knowing the business logic of adding a badge  
 
  name to a participant.
    [[File:RespControllerAwardBadge.png|1200px|]]
 
  [[File:RespControllerAwardBadge.png|1170px|]]


    [[File:AwardBadgeMethod.png|1200px|]]
  [[File:AwardBadgeMethod.png|1170px|]]
      
      
E. '''cake.rb'''  <br>
E. '''cake.rb'''  <br>
1. ''store_total_cake_score''
  '''store_total_cake_score'''
   This method clearly exposes the logic of retrieving the score for a cake question in the ''response_controller.rb'' class which is an incorrect design.   
   This method clearly exposes the logic of retrieving the score for a cake question in the ''response_controller.rb'' class which is an incorrect  
  design.   
   So, a model class method ''get_total_score_for_questions'' is added and the same logic of iterating over the Cake questions is moved there.
   So, a model class method ''get_total_score_for_questions'' is added and the same logic of iterating over the Cake questions is moved there.
   Return results for this method remains same here.
   Return results for this method remains same here.
 
   [[File:Cake.png|1170px|]]
   [[File:Cake.png|1200px|]]


F. '''response.rb'''  <br>
F. '''response.rb'''  <br>
1. ''calibration_results_info''
  '''calibration_results_info'''
    This method in ''response.rb'' model is completely removed because this method only adds more complexity to the model class.
  This method in ''response.rb'' model is completely removed because this method only adds more complexity to the model class.
    We refactored the method ''show_calibration_results_for_student'' in the ''response_controller.rb'' class.  
  We refactored the method ''show_calibration_results_for_student'' in the ''response_controller.rb'' class.  
    Previously, ''calibration_results_info'' was just doing nothing but getting responses using ''calibration_response_map_id'' and ''review_response_map_id'', and querying ''AssignmentQuestionnaire''.
  Previously, ''calibration_results_info'' was just doing nothing but getting responses using ''calibration_response_map_id'' and  
    Both of these operations can be done by using ''find'' method of model class and adding model class ''get_questions_by_assignment_id'' method.
  ''review_response_map_id'', and querying ''AssignmentQuestionnaire''.
  Both of these operations can be done by using ''find'' method of model class and adding model class ''get_questions_by_assignment_id'' method.


2. ''create_or_get_response''
  '''create_or_get_response'''
    Method name ''populate_new_response'' is changed to ''create_or_get_response'' to convey the functionality of the method.
  Method name ''populate_new_response'' is changed to ''create_or_get_response'' to convey the functionality of the method.
      
      
    [[File:Create_or_get_response.png|1200px|]]
  [[File:Create_or_get_response.png|1170px|]]
 
G. '''Code Climate issues''' <br>
  '''Update'''
  Made changes to split a line into multiple lines because it is too long.
  [[File:Update_line_long.png|1170px|]]
 
  '''Create'''
  Made changes to split a line into multiple lines because it is too long.
  [[File:Create_line_long.png|1170px|]]
 
== Testing ==
This is a refactoring project, so we have refactored response controller while ensuring that existing tests don't break.
=== Running Tests ===
<pre>
  rspec spec/controllers/response_controller_spec.rb
</pre>
 
[[File:Resp_test_p1.jpeg|800px|]]
[[File:Resp_test_p2.jpeg|800px|]]
 
=== UI Testing ===
1) Login using the following details. User Name: student432 Password: password


3. ''set_response''
[[File:resp_p1.png|800px|]]
    @response and @map instance variables are initialized the same way across multiple methods.
    In order to avoid duplication, we created a before_action action which will be called first before calling the corresponding methods.


    [[File:Set_response_method.png|1200px|]]
2) Click on the assignment Design Exercise pertaining to course CSC/ECE 517, Spring 2017


G. '''Code Climate issues''' <br>
[[File:resp_p2.png|800px|]]
1. ''Update'' <br>
    <p> Made changes to split a line into multiple lines because it is too long. </p>


    [[File:Update_line_long.png|1200px|]]
3) Click on other's work
2. ''Create'' <br>
    <p> Made changes to split a line into multiple lines because it is too long. </p>


    [[File:Create_line_long.png|1200px|]]
[[File:resp_p3.png|800px|]]


4) Click on show calibration results, verify if the calibration results are displayed on the UI.


== Testing ==
[[File:resp_p4.png|800px|]]


=== Running Tests ===
[[File:resp_p5.png|800px|]]
<pre>
  rspec spec/models/responses_spec.rb
</pre>
<pre>
  rspec spec/controllers/questionnaires_controller_spec.rb
</pre>


==GitHub links and Pull Request==
==GitHub links and Pull Request==

Latest revision as of 22:28, 27 March 2022

This page details project documentation for the CSC/ECE 517 Spring 2022, E2218 refactor response_controller.rb project.


Background

A response is an object that is created when someone fills out a review rubric, such as when one writes a review, gives feedback to a reviewer, or fills out a survey. Responses to the individual rubric items are kept in Answer objects; each Answer object has a response_id to say what Response it is part of. Since response_controller needs to work with many kinds of responses, its code is pretty general. It is not the worst controller in the system, but it would be much clearer if its method names were more descriptive of what they do.

Mentor

Ed Gehringer, efg@ncsu.edu

Team Members

  • Sai Naga Vamshi Chidara (schidar@ncsu.edu)
  • Akhil Kumar Mengani (amengan@ncsu.edu)
  • Samson Reddy Mulkur (smulkur@ncsu.edu)

Problem Statement

1) After providing a review of a particular project, calibration results are useful to differentiate the review given by the reviewer and that of the instructor's review for the same project. The main problem solved is to refactor the show_calibration_results_for_student method in the response controller. This method is called when the user clicks on show calibration results as shown in the UI Testing section.

Inside the show_calibration_results_for_student method, a call to the calibration_results_info method is made which is part of the response model. This particular method is returning three values calibration_response, review_response, questions. In general, it is not a good approach to return the array of three values from the model method as the controller method has no idea what would be the return value from the model method. We have done refactoring for this which is explained in the Modified files and Refactorization section.

2) In the Save method inside the response controller, changes are made which are specific to awarded badge model. But those changes should be part of a method present inside the awarded badge model and the controller should call that method. Changes are made for the same which are described in the below section.

3) There are code climate issues that are being fixed and are described in detail in the below section.

Modified Files and Refactorization

A. response_controller.rb

  authorize_show_calibration_results
  This method is used to authorize if the user(reviewer) is allowed to view the calibration results for the corresponding 
  calibration_response_map_id and review_response_map_id 
  passed as parameters in the URL.
  Ex: https://expertiza.ncsu.edu/response/show_calibration_results_for_student?calibration_response_map_id=165688&review_response_map_id=165790
  When someone tries to change the map_id's, the request should be authorized to check if the user can access the calibration result or not. 
  set_response
  @response and @map instance variables are initialized the same way across multiple methods.
  In order to avoid duplication, we created a before_action action which will be called first before calling the corresponding methods.
  


B. response_helper.rb

  Following methods are moved from the response_controller.rb class to response_helper.rb class since these methods are called from the 
  controller action methods.
   i) current_user_is_reviewer 
   ii) sort_questions
   iii) store_total_cake_score
   iv) set_content 
   

C. assignment_questionnaire.rb

  get_questions_by_assignment_id
  Parameters : assignment_id 
  When a user clicks on 'calibration results' from UI, questions in the assignment that has been reviewed are displayed.
  A new method is added in the AssignmentQuestionnaire model class that returns the appropriate questions by querying the 
  AssignmentQuestionnaire using the assignment_id.
  This method is added to separate the business logic of retrieving the questions from the controller class.
    
  

D. awarded_badge.rb

  award_badge
  Parameters :  participant_id, badge_name
  This method adds the badge_name to a specific participant_id in the AwardedBadges table. 
  A separate method is created in the AwardedBadges model class to avoid the controller class from knowing the business logic of adding a badge 
  name to a participant.
  
  
  
   

E. cake.rb

  store_total_cake_score
  This method clearly exposes the logic of retrieving the score for a cake question in the response_controller.rb class which is an incorrect 
  design.  
  So, a model class method get_total_score_for_questions is added and the same logic of iterating over the Cake questions is moved there.
  Return results for this method remains same here.
  

F. response.rb

  calibration_results_info
  This method in response.rb model is completely removed because this method only adds more complexity to the model class.
  We refactored the method show_calibration_results_for_student in the response_controller.rb class. 
  Previously, calibration_results_info was just doing nothing but getting responses using calibration_response_map_id and 
  review_response_map_id, and querying AssignmentQuestionnaire.
  Both of these operations can be done by using find method of model class and adding model class get_questions_by_assignment_id method.
  create_or_get_response
  Method name populate_new_response is changed to create_or_get_response to convey the functionality of the method.
    
  

G. Code Climate issues

  Update
  Made changes to split a line into multiple lines because it is too long.
  
  
  Create
  Made changes to split a line into multiple lines because it is too long.
  

Testing

This is a refactoring project, so we have refactored response controller while ensuring that existing tests don't break.

Running Tests

  rspec spec/controllers/response_controller_spec.rb

UI Testing

1) Login using the following details. User Name: student432 Password: password

2) Click on the assignment Design Exercise pertaining to course CSC/ECE 517, Spring 2017

3) Click on other's work

4) Click on show calibration results, verify if the calibration results are displayed on the UI.

GitHub links and Pull Request

Link to Expertiza repository: here

Link to the forked repository: here

Link to Pull Request: here