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.

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 views/suggestion/show.html.erb and 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

Test Cases

Below test cases were added/updated to ensure the correctness of our work.

<ADD TEST CASE LOGIC>

References

• Our Deployed Expertiza
• Expertiza on GitHub

Team Members

• Varun Varadarajan (vvarada2)
• Dylan Jordan (dtjordan)
• Santiago Sepulveda (sasepulv)
• Mentor: Sanket Pai