CSC/ECE 517 Fall 2020 - E2069. refactor suggestions controller
Background
The suggestions_controller.rb of Expertiza is involved in creating/editing suggestions topic suggestions for a project and involved approvals. Students can submit suggestions and work on them if approved.
Motivation
We intend to improve the readability and code quality of the suggestions_controller.rb. Since this controller has methods that must actually be done by other classes (owing to Single Responsibility feature of the SOLID principles), we have realized the need to move these methods to their respective classes.
Issues to Fix
1. Move the method create_new_team (on line 94) to a more appropriate class. (Ex: Team.rb or AssignmentTeam.rb)
2. Move the method approve to SignupTopic.rb, since it is modifying fields belonging to SignupTopic directly.
3. There are two methods named similarly: approve and approve_suggestion. Their functionality is not clear at first glance. Rename them so that they more accurately reflect their purpose. Add appropriate comments.
4. Move the send_email method to the Mailer class in app/mailers/mailer.rb.
5. In views/suggestion/show.html.erb and views/suggestion/student_view.html.erb, there seems to be a DRY violation which needs to be fixed. Merge both files into a single view file in order to fix the DRY problem.
Files Modified
- app/controllers/teams_controllers.rb
- app/models/sign_up_topic.rb
- app/mailers/mailer.rb
- views/suggestion/show.html.erb
Code Review
Issue: Move the method create_new_team to a more appropriate class.
This method was moved to teams_controller.rb since it is appropriate.
We pass the required variables inside the scope of this function as method parameters. Then we call the new method as a method of the Team class.
Issue: Move the method approve to SignupTopic.rb, since it is modifying fields belonging to SignupTopic directly.
First, we moved everything in the approve method from suggestion_controller having to do with a sign up topic to its own new method in sign_up_topic.rb now called new_topic_from_assignment. In doing so, we also delete the entire approve method from suggestion_controller, since refactoring for that method happens concurrently as a result of another issue in this project.
Issue:There are two methods named similarly: approve and approve_suggestion. Their functionality is not clear at first glance. Rename them so that they more accurately reflect their purpose. Add appropriate comments.
Here, we changed the name of "approve_suggestion" to "approve_suggestion_and_notify", since it really seems like this method is both to handle approving a suggestion as well as calling the notification method from elsewhere in the controller class. We also add in the first part of the original "approve" method here, so that we can set the instance variables that suggestion_controller might need (@user_id, @team_id, and @topic_id) as well as calling our newly created new_topic_from_suggestion method in SignUpTopic with the proper suggestion passed as a parameter.
Issue: Move the send_email method to the Mailer class.
This method was moved to the Mailer.rb file which seems to have many built-in functionalities for mail services.
Next, the required variables for this method were passed as parameters from suggestion_controller and is called as a method of Mailer class
Issue: Merge app/views/suggestion/show.html.erb and app/views/suggestion/student_view.html.erb into one view.
Modify student_view method in suggestion controller to provide @current_role_name and render show template
Migrate code from student_view.html.erb to show.html.erb and wrap in an if statement that will display view only if user role is a student.
Move the rest of the remaining code from the original show template to the else block
Delete the student_view.html.erb file since all of it's contents are contained within the show.html.erb file.
How to Check
Since this is a refactor project i.e. only the code was changed, the expected functionality before and after, represent the same functionality.
1. Log in as instructor 2. Create a new assignment 3. Tick the 'Has Teams' checkbox 4. In the 'Topics' tab, enable the suggestions option 5. Go to home page 6. Assign this assignment to a student belonging to this instructor 7. Impersonate the student now 8. Let the student view this assignment (may need to register too) 9. Student has an option to make a suggestion 10. Check that suggestion is shown after creation
Test Cases
Below test cases were added/updated to ensure the correctness of our work.
<ADD TEST CASE LOGIC>
References
Team Members
- Varun Varadarajan (vvarada2)
- Dylan Jordan (dtjordan)
- Santiago Sepulveda (sasepulv)
- Mentor: Sanket Pai