CSC/ECE 517 Fall 2013/oss ans
E807 : Refactoring and testing response_controller.rb
Project Description
The response controller creates, edits, and displays responses, that is, rubrics to be filled out, or filled-out rubrics. Our project requirement entailed the following things to be done.
- Reduce the method complexity in the response controller.
- Remove duplicated code.
- There were “custom_create” and “custom_update” methods for “custom” (multipart) rubrics. We already had create and update methods that were handling the normal rubrics. Our job was to use polymorphism in order to eliminate the custom methods.
Refactoring carried out
As per the project requirements the following work was carried out :
- The duplicated code has been refactored and put in a separate method which is called wherever required. This prevents redundancy.
- The variables set up in controller become automatically available to the views. Hence the repeated code from controller and view for finding the topic id signed up by a user/team has been deleted from controller and incorporated only in the response view. The following code snippet was removed from the response controller.
if !@map.contributor.nil? if @map.assignment.team_assignment? team_member = TeamsUser.find_by_team_id(@map.contributor).user_id @topic_id = Participant.find_by_parent_id_and_user_id(@map.assignment.id,team_member).topic_id else @topic_id = Participant.find(@map.contributor).topic_id end end if !@topic_id.nil? @signedUpTopic = SignUpTopic.find(@topic_id).topic_name end
Because , similar code existed in the response view
<% if !@map.contributor.nil?%> <%if @map.assignment.team_assignment? team_member = TeamsUser.find_by_team_id(@map.contributor).user_id topic_id = Participant.find_by_parent_id_and_user_id(@map.assignment.id,team_member).topic_id else%> <% topic_id = Participant.find(@map.contributor).topic_id%> <%end%> <%if !topic_id.nil?%> <h2>You are reviewing <%=SignUpTopic.find(topic_id).topic_name%></h2> <%end%>
- We created a new method in order to find the latest response thereby by eliminating the need to reuse the following code multiple times.
def latestResponseVersion #get all previous versions of responses for the response map. array_not_empty=0 @review_scores=Array.new @prev=Response.find_by_map_id(@map.id) for element in @prev array_not_empty=1 @review_scores << element end
- The custom create and custom update methods have been removed. Their functionality has been merged with create and update methods as was mentioned in the requirement specification. Depending on the type of rubric rendered, the create and update method does the necessary updations.
Testing Done
Integration tests for capybara were created for the scenarios describing admin login, creation of an assignment by admin and user login.
Future Work
Design Changes
- An even better way to handle the rubrics would be to do subclassing of the questionnaire model in order to create two sub classes the rating rubric and the multipart rubric. As per the rubric being rendered the appropriate class would be called. This method would be much more efficient than the current way the functionality is being handled also it would be more O-O like.
Further Refactoring
Given the current state of the response controller, further improvements can be done to the code:
- The controller still has some code that is only there for some specific assignments. This is bad design and this code needs to be removed.
- The purpose of saving method is not quite clear. It predominantly has the code for an assignment '562' which has been hard coded and can be removed.
- The rubric model has been created but has not been used anywhere in the code.
Appendix
Setup Process
- We cloned the repo from https://github.com/expertiza/expertiza into our local directory.
- After installing MySQL server we loaded the test-db.sql sample database into the application.
- Next we created our own users and a new assignment to understand the working of the response controller.
- First of all, an admin needs to create a minimum of two users who will participate in the same assignment.
- The admin needs to create one assignment and add participants to the assignment. Further details of the assignment need to be added as necessary such as due dates, review strategy etc.
- Review strategy can be auto selected or student selected. If the TA or instructor is manually assigning the assignments for review, it has to be “instructor selected”.
- Check the "Due dates" tab and check if the review is allowed at the current date. Update due dates so that the review date starts after the submission date is passed. Save dependencies.
- Add a signup sheet to the assignment. This sheet will display topics which the users can select to review.
- Now, login through user1 account and select an assignment. Then sign up for one topic from the list of available topics.
- After that, user1 needs to submit a hyperlink.
- Now, login through user2 credentials. User2 must select the assignment(of which it was made a participant by the admin).
- User2 must go to the student_task page and select the link "other's work" which will be enabled only if a particular topic of an assignment has review_allowed to be true, that means the deadline should be properly configured by the admin.
- After that, response view would be rendered to the reviewer.
- The reviewer would review the assignment submitted by user1. Once the review has been saved, user1 will have an option to provide feedback on the review.
- If meta reviews have been enabled by the admin, then the other participants will be able to see the reviews of other users.
Issues Encountered
- In order to get the application working properly we added several routes to the routes.rb file.
- Assignment submission status was not being changed in spite of changing the deadline due-dates in the edit assignment section for the admin.
External Links
- 1. Expertiza
- 2. VCL Link