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 portion of 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_suggestion. 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 only the code style/process was changed, the expected functionality before and after, will be the same. Student should be able to create a suggestion for an assignment, just like before.

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 (information previously provided by student_view.html.erb).

• The corresponding page for an instructor would appear as such.

Please refer to our short videos to get more context on what was done.

• Here is our first video demonstrating the same functionality is maintained after refactoring. This video demonstrates a suggestion being approved.

• Here is our other video demonstrating the ability to reject a suggestion.

NOTE TO TA: Our Travis CI build failed because of an error in Travis's config settings (bundle exec danger --verbose). Hence, we have included this video as a proof of our working logic and testing.

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 method for approved suggestions

This test ensures the working of the team creation method

References

• Our Deployed Expertiza
• Expertiza on GitHub

Team Members

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