CSC/ECE 517 Fall 2016/oss E1639: Difference between revisions

From Expertiza_Wiki
Jump to navigation Jump to search
mNo edit summary
No edit summary
 
(43 intermediate revisions by 3 users not shown)
Line 1: Line 1:
== '''E1639 - Refactor response_controller.rb''' ==
This wiki page describes changes made under E1639 OSS project assignment for Fall 2016, CSC/ECE 517.
 
This wiki page is for the description of changes made under E1555 OSS assignment for Fall 2015, CSC/ECE 517.
 
== Peer Review Information ==
 
For users intending to view the deployed Expertiza associated with this assignment, the credentials are below:
* Instructor login: username -> instructor6,  password -> password
* Student  login: username -> student4340,  password -> password
* Student login: username -> student4405,  password -> password


== Expertiza Background==
== Expertiza Background==


Expertiza is an educational web application created and maintained by the joint efforts of the students and  the faculty at NCSU. It’s an open source project developed on Ruby on Rails platform and it’s code is available on Github. It allows students to review each other’s work and improve their work upon this feedback.
Expertiza is a web application for educational purposes. It is an open source project based on Ruby on Rails framework. Expertiza has been created and maintained by faculty and students of NCSU. It helps teachers set up assignments for students who can then make submissions. Students can also review work of other students and give feedback to help incorporate improvements.
 
== Description of the current project ==
 
The following is an Expertiza based OSS project which deals primarily with the GradesController and GradesHelper. It focusses on refactoring some of the more complex methods, modifying some of the language to make it more Ruby friendly, removing some redundant code. The goal of this project is to attempt to make this part of the application easier to read and maintain.
 
== Files modified in current project ==
 
A controller and a helper file were modified for this project namely:<br/>
1. GradesController <br/>
2. GradesHelper <br/>
 
=== GradesController ===
This is a controller that helps students and instructors view grades and reviews, update scores, check for grading conflicts and calculate penalties. A couple of long and complex methods were refactored from this controller along with removal of some non-functional code and a few language changes to make it Ruby style.
Three methods in particular, namely conflict_notification ,calculate_all_penalties and edit were found to be too long and were in need of refactoring into smaller, easier to manage methods. Few more  compact methods were created for this purpose.
 
There were no existing test cases for the controller. We have added a spec file named 'grades_spec.rb' under the spec folder. As no changes were done for the model, no tests for the model were included.
 
=== GradesHelper ===
 
This is a helper class which contains methods for constructing a table(construct_table) and to check whether an assignment has a team and metareveiw(has_team_and_metareview)
 
== List of changes ==
We worked on the following work items(WIs)<br/>
WI1 : Refactor calculate_all_penalties method into smaller methods<br/>
WI2 : Move the repeated code in conflict_notification & edit methods to a separate method list_questions.<br/>
WI3 : Refactor the code as per the Ruby style guidelines and incorporate the good practices<br/>
WI4 : Test the conflict_notification method to test the changes made.<br/>
WI5 : Move the repeated code in view and view_my_scores methods to a separate method retrieve_questions
 
=== Solutions Implemented and Delivered ===
 
*Refactoring calculate_all_penalties method
 
This is used to calculate various penalty values for each assignment if penalty is applicable.


The following changes were made:
== Project Description ==


1. This method was very complex, performing too many functions within a single method and had to be broken into 3 smaller methods each having a more well defined function.
The following OSS project deals mainly with the ResponseController. The goal of this project is make the code more readable, maintainable and to improve elegance of the code. We would like ResponseController to adhere to the DRY principle.  
2. The following 3 methods were created after splitting the first method<br>
It focuses on refactoring some of the more complex methods and removing some redundant code. At present, ResponseController has methods that would better be located in other controllers. The project relocates such methods to its appropriate Controller class.
  i. calculate_all_penalties<br>
  ii. calculate_penatly_attributes<br>
  iii. assign_all_penalties<br>
3. Changes were also made to make the code follow ruby style.The language was made more ruby friendly.
4. Finally some redundant code was commented out as it was non-functional.


== ResponseController ==
   
   
Response controller manages the responses entered by users. When a user fills out any kind of rubric (review rubrics,author-feedback rubrics,teammate-review rubrics,quizzes,surveys), a response is generated. Responses come in different versions. Any time an author revises his/her work, and the reviewer reviews it again, a separate Response object is generated.Each Response object points to a particular ResponseMap, which provides details about reviewer, reviewee and reviewed entity.


Refactoring into smaller more specific methods:
== What needs to be done ==
1)  Refactor the update method which is too long and hard to read.<br/>
2)  Refactor the saving method to improve the case handling of selfReviewResponseMap<br/>
3)  Move new_feedback method from ResponseController to ReviewMappingController - ResponseController should only handle one kind of object<br/>
4)  Modified functional tests for new_feedback method<br/>
5)  Added functional test for update_method of response controller<br/>


[[File:Change6_new.png]]
== Files modified ==


Removal of non-functional code :
Following files were mainly modified for this project namely:<br/>
1. response_controller.rb <br/>
2. response_controller_spec.rb <br/>
3. review_mapping_controller.rb <br/>
4. review_mapping_controller_spec.rb <br/>
5. routes.rb <br/>
6. _reviews.html.erb <br/>
7. _scores_submitted_work.html.erb <br/>
8. _self_review.html.erb


[[File:Change5_new.png]]
== Solutions Implemented and Delivered ==


==='''Refactor the update method'''===


Change of language to make it more Ruby friendly:
Update method is called when any kind of response is edited by the user. This method was very long and unreadable. It was checking the type of @map and round parameter to find out questionnaire  using long if..elseif..else ladder, which did not look neat and elegant. There was a helper method 'set_questionnaire' which was being called indirectly by the edit method for retrieving the questionnaire.We called the same function. The intuition behind using this same method was that if it is an update, user is not filling a new rubric and the response object should be available.We can find the questionnaire from the question_id in answers.Hence, in this way the if-else block was eliminated by calling the already available function.
Similarly, there were lines of code,which were creating score if not found or updating if an entry exists. The function 'create_answers' does exactly the same. Abiding by the DRY principle, we called this function which made the update method look concise and simplified. <br/><br/> 
[[File:upate_method.png]]
<br/><br/><br/><br/>
[[File:update_rspec.png]]
<br/><br/>


[[File:Change1_new.png]]
==='''Refactor the saving method'''===


Saving method is called to save the response when a user edits any particular response or creates one. It was assigning params[:returs] to 'selfview' if the @map type is selfReponseMap. This params[:return] value was then further used in redirection method to handle the selfReviewResponseMap separately. We removed the assignment params[:return]= "selfreview" from save method. Rather we are directly passing the :return parameter value from edit and new methods of _self_review.html.erb, a view of SubmittedContent controller which handles the selfReview functionality.<br/>
<br/>
app -> controllers -> respose_controller.rb -> def saving <br/> <br/>
[[File:saving.png]]


*Move the redundant piece of code from conflict_notification & edit methods to a new method list_questions
app -> views -> submitted_content -> _self_review.html.erb<br/> <br/>
[[File:_self_review.png]]
<br/> <br/>


The conflict_notification method is used to help the instructors decide if one of the reviews are unfair or inaccurate.
==='''Move new_feedback method from ResponseController to ReviewMappingController'''===
This was again split into 2 methods with some part of the code which is repeated in another method  refactored into a new method.


[[File:Change3_new.png]]
new_feedback method is called when a user provides author feedback for any review. It was defined in the ResponseController earlier which was rather suspect. We moved the method to ReviewMappingController since ResponseController should only handle Response object, while new_feedback is dealing with FeedbackResponseMap object.<br/><br/>
[[File:response_controller_changes1.png]]
<br/><br/>
[[File:review_mapping_controller_changes1.png]]
<br/><br/>
[[File:new_feedback_html_changes1.png]]
<br/><br/>


== Testing ==


Refactored #Created a method which was a duplicate in conflict_notification and edit methods
==='''Modified functional tests for new_feedback method'''===
 
[[File:Change4_new.png]]


edit method:
We modified the test cases to accommodate changes of the new_feedback method and created new rspec file ''review_mapping_controller_rspec.rb''<br/>


This method is used to edit the questionnaires. This method again has code which is repeated in the conflict_notification method and thus the repeated section was split into a new method.
==='''Added functional test for update method'''===


[[File:Change2_new.png]]
We added new test case for update method to test if it located the requested response.<br/>
<br/><br/> 
[[File:update_rspec_final.png]]
<br/><br/>


New method:
==='''UI testing'''===
Refactored #Created a method which was a duplicate in conflict_notification and edit methods
 
[[File:Change4_new.png]]
 
Similar refactoring was performed to obtain the retrieve_questions method:
 
[[File:Latest1.png]]
 
This is the new method created after the above refactoring:
 
[[File:Latest2.png]]
 
== Testing Details==
 
=== RSpec ===
There were no existing test cases for the GradesController. We have added a new spec file 'grades_spec.rb' which covers testing scenario for the newly added method. The specs were run on the previous and current files and they return the same results implying that the refactored code does not break anything.
As the model was not changed, no test cases were added for the model.
 
=== UI Testing ===


Following steps needs to be performed to test this code from UI:<br/>
Following steps needs to be performed to test this code from UI:<br/>
1. Login as instructor. Create a course and an assignment under that course.<br/>
'''Testing the update method: <br/>'''
2. Keep the has team checkbox checked while creating the assignment. Add a grading rubric to it. Add at least two students as participants to the assignment.<br/>
<br/>
3. Create topics for the assignment.<br/>
Click on [https://www.youtube.com/watch?v=Xof869v6Eg8] to view testing screencast for testing the update method <br/>
4. Sign in as one of the students who were added to the assignment.<br/>
1. Login as instructor. Create a course and an assignment under that course<br/>
5. Go to the assignment and sign up for a topic.<br/>
2. Add say, two students as participants to the assignment<br/>
6. Submit student's work by clicking 'Your work' under that assignment.<br/>
3. Create topics for the assignment<br/>
7. Sign in as a different student which is participant of the assignment.<br/>
4. Sign in as one of the students who were added to the assignment<br/>
8. Go to Assignments--><assignment name>-->Others' work (If the link is disabled, login as instructor and change the due date of the assignment to current time).<br/>
5. Go to the assignment and sign up for a topic<br/>
9. Give reviews on first student's work.<br/>
6. Submit student's work by clicking 'Your work' under that assignment<br/>
10. Login as instructor or first student to look at the review grades.<br/>
7. Sign in as a different student which is participant of the assignment <br/>
 
8. Give review on first student's work and simply click on save <br/>
 
9. Later click on edit and change the review. Click on submit <br/>
== Scope for future improvement ==
10. When you click on view, verify the changes made in step 10. If all changes are intact, update method worked successfully <br/>
1. The construct_table method in GradesHelper is not used anywhere. It has no reference in the project. So we feel it can be safely removed.<br/>
<br/>
2. The has_team_and_metareview? method in GradesHelper can be broken down into separate methods, one each for team and metareview. This will provide improved flexibility. It needs some analysis though, as both the entities(team & metareview) are currently checked in conjuction from all the views they are referenced from.
'''Testing the new_feedback method: <br/>'''
<br/>
Click on [https://www.youtube.com/watch?v=CQXjpPaXWBo] to view testing screencast for testing the new_feedback method <br/>
11. Login as the first student to view feedback given in step 10<br/>
12. Click on "Give Feedback" to give author feedback ( feedback on the review quality) <br/>
13. You would be redirected to a form. Fill in the author feedback and save your changes <br/>
14. View your feedback for the review. If your changes are intact, new_feedback method worked successfully <br/>

Latest revision as of 02:21, 5 November 2016

This wiki page describes changes made under E1639 OSS project assignment for Fall 2016, CSC/ECE 517.

Expertiza Background

Expertiza is a web application for educational purposes. It is an open source project based on Ruby on Rails framework. Expertiza has been created and maintained by faculty and students of NCSU. It helps teachers set up assignments for students who can then make submissions. Students can also review work of other students and give feedback to help incorporate improvements.

Project Description

The following OSS project deals mainly with the ResponseController. The goal of this project is make the code more readable, maintainable and to improve elegance of the code. We would like ResponseController to adhere to the DRY principle. It focuses on refactoring some of the more complex methods and removing some redundant code. At present, ResponseController has methods that would better be located in other controllers. The project relocates such methods to its appropriate Controller class.

ResponseController

Response controller manages the responses entered by users. When a user fills out any kind of rubric (review rubrics,author-feedback rubrics,teammate-review rubrics,quizzes,surveys), a response is generated. Responses come in different versions. Any time an author revises his/her work, and the reviewer reviews it again, a separate Response object is generated.Each Response object points to a particular ResponseMap, which provides details about reviewer, reviewee and reviewed entity.

What needs to be done

1) Refactor the update method which is too long and hard to read.
2) Refactor the saving method to improve the case handling of selfReviewResponseMap
3) Move new_feedback method from ResponseController to ReviewMappingController - ResponseController should only handle one kind of object
4) Modified functional tests for new_feedback method
5) Added functional test for update_method of response controller

Files modified

Following files were mainly modified for this project namely:
1. response_controller.rb
2. response_controller_spec.rb
3. review_mapping_controller.rb
4. review_mapping_controller_spec.rb
5. routes.rb
6. _reviews.html.erb
7. _scores_submitted_work.html.erb
8. _self_review.html.erb

Solutions Implemented and Delivered

Refactor the update method

Update method is called when any kind of response is edited by the user. This method was very long and unreadable. It was checking the type of @map and round parameter to find out questionnaire using long if..elseif..else ladder, which did not look neat and elegant. There was a helper method 'set_questionnaire' which was being called indirectly by the edit method for retrieving the questionnaire.We called the same function. The intuition behind using this same method was that if it is an update, user is not filling a new rubric and the response object should be available.We can find the questionnaire from the question_id in answers.Hence, in this way the if-else block was eliminated by calling the already available function. Similarly, there were lines of code,which were creating score if not found or updating if an entry exists. The function 'create_answers' does exactly the same. Abiding by the DRY principle, we called this function which made the update method look concise and simplified.







Refactor the saving method

Saving method is called to save the response when a user edits any particular response or creates one. It was assigning params[:returs] to 'selfview' if the @map type is selfReponseMap. This params[:return] value was then further used in redirection method to handle the selfReviewResponseMap separately. We removed the assignment params[:return]= "selfreview" from save method. Rather we are directly passing the :return parameter value from edit and new methods of _self_review.html.erb, a view of SubmittedContent controller which handles the selfReview functionality.

app -> controllers -> respose_controller.rb -> def saving

app -> views -> submitted_content -> _self_review.html.erb



Move new_feedback method from ResponseController to ReviewMappingController

new_feedback method is called when a user provides author feedback for any review. It was defined in the ResponseController earlier which was rather suspect. We moved the method to ReviewMappingController since ResponseController should only handle Response object, while new_feedback is dealing with FeedbackResponseMap object.







Testing

Modified functional tests for new_feedback method

We modified the test cases to accommodate changes of the new_feedback method and created new rspec file review_mapping_controller_rspec.rb

Added functional test for update method

We added new test case for update method to test if it located the requested response.




UI testing

Following steps needs to be performed to test this code from UI:
Testing the update method:

Click on [1] to view testing screencast for testing the update method
1. Login as instructor. Create a course and an assignment under that course
2. Add say, two students as participants to the assignment
3. Create topics for the assignment
4. Sign in as one of the students who were added to the assignment
5. Go to the assignment and sign up for a topic
6. Submit student's work by clicking 'Your work' under that assignment
7. Sign in as a different student which is participant of the assignment
8. Give review on first student's work and simply click on save
9. Later click on edit and change the review. Click on submit
10. When you click on view, verify the changes made in step 10. If all changes are intact, update method worked successfully

Testing the new_feedback method:

Click on [2] to view testing screencast for testing the new_feedback method
11. Login as the first student to view feedback given in step 10
12. Click on "Give Feedback" to give author feedback ( feedback on the review quality)
13. You would be redirected to a form. Fill in the author feedback and save your changes
14. View your feedback for the review. If your changes are intact, new_feedback method worked successfully

Retrieved from "https://wiki.expertiza.ncsu.edu/index.php?title=CSC/ECE_517_Fall_2016/oss_E1639&oldid=104592"