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 assignment_team.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 AssignmentTeam 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 an instructor.
- Login with username instructor6 and password password.
2. Create a new assignment.
- Click the Assignments link.
- Click the + icon.
3. Fill in the following fields in the 'General' tab.
- Provide Assignment name.
- Check the Has Teams checkbox and set maximum number of members to greater than or equal to 1.
- Click the Create button.
- Page will reload and have an additional checkbox, Has topics?. Check it off and click the Save button.
4. Make the following selections in the Topics tab.
5. Assign this assignment to a student belonging to this instructor
- Select Add participant link within the Other stuff tab. This will bring you to a new page.
- Find student of interest (we recommend using student1876). Select Add button.
- You should receive confirmation of successfully adding the student to the assignment.
6. Impersonate the assigned student.
- Select from the Manage dropdown menu Impersonate User.
- Fill in field with student username and select Impersonate button.
7. Let the student view this assignment (may need to register too)
- Select the created assignment from the table seen at the bottom of the page.
- Select Suggest a topic link.
- Create new suggestion, provide a title, description, and click the Submit button.
- Select View link for the suggestion created.
- This will bring the following page that
8. Student has an option to make a suggestion
10. Check that suggestion is shown after creation
Test Cases
The demonstration shown in the above section is how we tested the functionality and behavior of our changes. The spec tests below will test that the moved methods work independently.
This test ensures the working of the mailer & approve_suggestion methods
This test ensures the working of the team creation methods
References
Team Members
- Varun Varadarajan (vvarada2)
- Dylan Jordan (dtjordan)
- Santiago Sepulveda (sasepulv)
- Mentor: Sanket Pai (sgpai2)