CSC/ECE 517 Fall 2022 - E2250. Refactor suggestion controller.rb

From Expertiza_Wiki
Revision as of 11:26, 25 October 2022 by Vrcholay (talk | contribs)
Jump to navigation Jump to search

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.
  • Increased the test coverage of the suggestion controller by adding 3 more tests: 'adds a participant is not successfull', 'rejecting a suggestion is not successfull', 'accept a suggestion is not successfull'

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: Refactor 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
  • Problem 3: Rename methods approve and approve_suggestion to correctly reflect their functionalities.
def approve
  """
    method content
  """
end

def approve_suggestion
  """
    method content
  """
end
  • Solution: Renamed approve method to approve_suggestion and approve_suggestion to approve_suggestion_and_notify
def approve_suggestion
  """
    method content
  """
end

def approve_suggestion_and_notify
  """
    method content
  """
end
  • Problem 4: Move the send_email method from suggestion_controller to Mailer class in app/Mailer/mailer.rb file and change its name.
def send_email
    proposer = User.find_by(id: @user_id)
    if proposer
      teams_users = TeamsUser.where(team_id: @team_id)
      cc_mail_list = []
      teams_users.each do |teams_user|
        cc_mail_list << User.find(teams_user.user_id).email if teams_user.user_id != proposer.id
      end
      Mailer.suggested_topic_approved_message(
        to: proposer.email,
        cc: cc_mail_list,
        subject: "Suggested topic '#{@suggestion.title}' has been approved",
        body: {
          approved_topic_name: @suggestion.title,
          proposer: proposer.name
        }
      ).deliver_now!
    end
  end
  • Solution: Moved the send_email method from suggestion_controller to Mailer class in app/Mailer/mailer.rb file. Also changed the name of the method to notify_suggestion_approval.
def notify_suggestion_approval(used_id, team_id, suggestion_title)
    proposer = User.find_by(id: user_id)
    if proposer
      teams_users = TeamsUser.where(team_id: team_id)
      cc_mail_list = []
      teams_users.each do |teams_user|
        cc_mail_list << User.find(teams_user.user_id).email if teams_user.user_id != proposer.id
      end
      suggested_topic_approved_message(
        to: proposer.email,
        cc: cc_mail_list,
        subject: "Suggested topic '#{suggestion_title}' has been approved",
        body: {
          approved_topic_name: suggestion_title,
          proposer: proposer.name
        }
      ).deliver_now!
    end
 end
  • Problem 5: In views/suggestion/show.html.erb and views/suggestion/student_view.html.erb, there is a DRY violation which needs to be fixed.
  • Solution: Merge views/suggestion/show.html.erb and views/suggestion/student_view.html.erb into one file to fix the DRY problem.
  • Problem 6: Increase the test coverage of the controller and functionality by adding more tests.
  • Solution: Increase the test coverage of the suggestion controller by adding 3 more tests: 'adds a participant is not successfull', 'rejecting a suggestion is not successfull', 'accept a suggestion is not successfull'
it 'adds a participant is not successfull' do
      allow_any_instance_of(SuggestionComment).to receive(:save).and_return(false)
      request_params = { id: 1, suggestion_comment: { vote: 'Y', comments: 'comments' } }
      user_session = { user: instructor }
      get :add_comment, params: request_params, session: user_session, xhr: true
      expect(flash[:error]).to eq 'There was an error in adding your comment.'
end
it 'rejecting a suggestion is not successfull' do
        allow_any_instance_of(Suggestion).to receive(:update_attribute).and_return(false)
        request_params = { id: 1, reject_suggestion: true }
        user_session = { user: instructor }
        get :submit, params: request_params, session: user_session, xhr: true
        expect(flash[:error]).to eq 'An error occurred when rejecting the suggestion.'
end
it 'accept a suggestion is not successfull' do
        allow(SignUpTopic).to receive(:new_topic_from_suggestion).and_return('failed')
        allow_any_instance_of(SuggestionController).to receive(:notification).and_return(true)
        request_params = { id: 1, approve_suggestion: true }
        user_session = { user: instructor }
        get :submit, params: request_params, session: user_session, xhr: true
        expect(flash[:error]).to eq 'An error occurred when approving the suggestion.'
end

Automated Testing using RSPEC

Go to the expertiza source code root path and use following command to run test:

$ rspec spec/contollers/suggestion_controller_spec.rb

Here is the operation video link:

https://youtu.be/qzkdNmTnAIc

Test Plan

Coverage

Our coverage increased 8.04%.

  • Before Changes

  • After Changes


Testing from UI

Here is the VCL link:

http://152.7.176.119:8080/

For users intending to view the deployed Expertiza associated with this assignment, the credentials are below:

  • Instructor login: username -> instructor6, password -> password (Use this account for Testing)
  • Student login: username -> student4340, password -> password
  • Student login: username -> student4405, password -> password

1. After logging in as Instructor, click "Manage instructor content" -> "Assignment":

  

2. Select the "Assignment" tab, in this example we search "textbook" as figure shows.

  

3. Choose a item and click "view suggestion" icon. In this example we use "Wiki textbook 2":

  

4. Click the "view" link in which tuple the Status is "Initiated":

  

5. Now we can see the Suggestion page with "Approve suggestion" and "Reject suggestion" butttons:

  

References

  1. Expertiza on GitHub
  2. GitHub Project Repository Fork
  3. The live Expertiza website
  4. VCL link (will expire on Nov 24th 2022)
  5. Expertiza project documentation wiki
  6. Rspec Documentation
  7. Clean Code: A handbook of agile software craftsmanship. Author: Robert C Martin