CSC/ECE 517 Fall 2022 - E2250. Refactor suggestion controller.rb
This page provides a description of the Expertiza based OSS project.
About Expertiza
Expertiza is an open source project based on Ruby on Rails framework. Expertiza allows the instructor to create new assignments and customize new or existing assignments. It also allows the instructor to create a list of topics the students can sign up for. Students can form teams in Expertiza to work on various projects and assignments. Students can also peer review other students' submissions. Expertiza supports submission across various document types, including the URLs and wiki pages.
Problem Statement
The following tasks were accomplished in this project:
- Added more comments to the controller, briefly explain the functionality of custom methods.
- Refactored the notification method to make it simpler by simplifying the control logic and reducing the function's complexity.
- Renamed methods approve and approve_suggestion to correctly reflect their functionalities.
- Moved the send_email method from suggestion_controller to Mailer class in app/Mailer/mailer.rb file. Renamed the method to notify_suggestion_approval.
- Merged views/suggestion/show.html.erb and views/suggestion/student_view.html.erb into one file to fix the DRY problem.
Problems and Solutions
- Problem 1: Add more comments to the controller, briefly explain the functionality of custom methods.
- For example for notification method
def notification """ method content """ end
- Solution: The approach we have taken is to add comment lines to all methods present in the suggestion_controller.rb file.
#will provie notification/email based on the suggestion being approved or not #will create and assign team if user is not in any team def notification """ method content """ end
- Problem 2: Refactored the notification method to make it simpler by simplifying the control logic and reducing the function's complexity.
def notification if @suggestion.signup_preference == 'Y' if @team_id.nil? new_team = AssignmentTeam.create(name: 'Team_' + rand(10_000).to_s, parent_id: @signuptopic.assignment_id, type: 'AssignmentTeam') new_team.create_new_team(@user_id, @signuptopic) else if @topic_id.nil? # clean waitlists SignedUpTeam.where(team_id: @team_id, is_waitlisted: 1).destroy_all SignedUpTeam.create(topic_id: @signuptopic.id, team_id: @team_id, is_waitlisted: 0) else @signuptopic.private_to = @user_id @signuptopic.save # if this team has topic, Expertiza will send an email (suggested_topic_approved_message) to this team send_email end end else # if this team has topic, Expertiza will send an email (suggested_topic_approved_message) to this team send_email end end
- Solution: Remove the nested if-else structure and fit all the functionalities into one if-elsif-else-end structure
def notification if @suggestion.signup_preference == 'Y' and @team_id.nil? new_team = AssignmentTeam.create(name: 'Team_' + rand(10_000).to_s, parent_id: @signuptopic.assignment_id, type: 'AssignmentTeam') new_team.create_new_team(@user_id, @signuptopic) elsif @suggestion.signup_preference == 'Y' and !@team_id.nil? and @topic_id.nil? # clean waitlists SignedUpTeam.where(team_id: @team_id, is_waitlisted: 1).destroy_all SignedUpTeam.create(topic_id: @signuptopic.id, team_id: @team_id, is_waitlisted: 0) elsif @suggestion.signup_preference == 'Y' and !@team_id.nil? and !@topic_id.nil? @signuptopic.private_to = @user_id @signuptopic.save # if this team has topic, Expertiza will send an email (suggested_topic_approved_message) to this team Mailer.notify_suggestion_approval(@used_id, @team_id, @suggestion.title) else # if this team has topic, Expertiza will send an email (suggested_topic_approved_message) to this team Mailer.notify_suggestion_approval(@used_id, @team_id, @suggestion.title) end end