CSC/ECE 517 Fall 2016/OSS E1637

E1637: Refactor penalty_helper.rb and late_policies_controller.rb

Expertiza

Expertiza is an educational web application built on Ruby on Rails framework. It provides functionality for the instructor to create assignments for a specific course. The students can sign up for a list of topics created by the instructor. Expertiza allows students to form teams and work together. Students can provide feedback to assignments and projects submitted by other students. The students can also provide reviews about each individual member in their team as well. The numerous functionalities provided by expertiza makes the process of assignment submission, feedback and management much simpler and for the students and the instructors.

Motivation

This project deals with LatePoliciesController and PenaltyHelper files. It focuses on calculating a penalty which would be deducted from the total score obtained by the student if the student submits the assignment or review past the specified deadline. The goal of our project is to make the task of calculating and deducting penalties function perfectly and also refactor the code to make it more intuitive and modular.

Files Modified

The files modified for the penalty functionality to work are as follows,

Controllers

LatePolicyController

This controller contains the functions to create, update and delete a late policy. The instructor can create a late policy for any assignment, where in the instructor can specify the points per unit and the maximum penalty that can be applied for a particular assignment. The create and the update functions in this module were very long and it had to be refactored so that the code was modular.

GradesController

In grades controller, the total penalty was not getting deducted from the total score due to the mismatch of conditions. The conditions were modified in this controller for the penalty to be deducted from the total score obtained.

Helpers

PenaltyHelper

This helper file has functions to calculate submission, review and metareview penalties. This function was not returning the values of the calculated penalty as required and modifications were made so that the correct penalty values are returned.

Views

In the current implementation, the penalty deducted is not displayed when a student views scores. The views were modified so that the deducted penalties and total score are displayed appropriately. The list of views modified are as follows,

• _participant.html.erb
• _participant_charts.html.erb
• _participant_title.html.erb

Rspec Testing

LatePolicy Model Testing

Rspec tests were written to test the functionality of the LatePolicy model. The tests ensure that the attributes in the model obtain the correct value. In addition, the tests also ensure that none of the fields are empty and ensures the maximum penalty and penalty per unit is a positive number which lies in a specified range.

List of Tasks

• T1: Refactor the create function in LatePoliciesController to make the code more modular
• T2: Refactor the update function in LatePoliciesController to make the code more modular
• T3: Modify the PenaltyHelper to calculate and return the correct penalty value
• T4: Modify the GradesController to deduct the total penalty from the total score
• T5: Modify the ViewScores page to display the penalties deducted
• T6: Test the working of the LatePolicy model by writing necessary RSpec tests

Implementation of Tasks

T1: Refactor create function in LatePoliciesController

The create function performed various checks on the parameters passed from the form instead of calling appropriate functions to do them. The same checks were performed by the update function. This violated the DRY principle. To overcome this discrepancy, two functions were created namely, validate_penalty_unit and validate_duplicate_policy which perform the required checks.

validate_penalty_unit

This function checks that the penalty per unit is less than the maximum penalty. If this function returns fall, the LatePolicy will not be created.

  private def validate_penalty_unit(params)
    penalty_valid = false
    if params[:late_policy][:max_penalty].to_i < params[:late_policy][:penalty_per_unit].to_i
      flash[:error] = "The maximum penalty cannot be less than penalty per unit."
    else
      penalty_valid = true
    end
    penalty_valid
  end

validate_duplicate_policy

This function checks that another policy with the same name created by a different instructor does not exist. Duplicate policy names are permissable if created by the same instructor.

  private def validate_duplicate_policy(params)
    not_duplicate = false
    @late_policy = LatePolicy.where(policy_name: params[:late_policy][:policy_name])
    if !@late_policy.nil? && !@late_policy.empty?
      @late_policy.each do |p|
          if p.instructor_id != instructor_id
            flash[:error] = "A policy with the same name already exists."
            return not_duplicate
          end
      end
    end
    not_duplicate = true
    not_duplicate
  end

T2: Refactor the update function in LatePoliciesController

The update function performs the same checks as the create function. The check statements are replaced by the appropriate calls to validate_penalty_unit and validate_duplicate_policy. In addition, when the late policy is updated, the corresponding penalty calculations have to be updated for students enrolled in assignments using the corresponding late policy. So a new function called values_update is introduced which performs the necessary task.

values_update

  private def values_update(params)
    @penaltyObjs = CalculatedPenalty.all
    @penaltyObjs.each do |pen|
      @participant = AssignmentParticipant.find(pen.participant_id)
      @assignment = @participant.assignment
      next unless @assignment.late_policy_id == @penalty_policy.id
      @penalties = calculate_penalty(pen.participant_id)
      @total_penalty = (@penalties[:submission] + @penalties[:review] + @penalties[:meta_review])
      if pen.deadline_type_id.to_i == 1
        {penalty_points: @penalties[:submission]}
        pen.update_attribute(:penalty_points, @penalties[:submission])
      elsif pen.deadline_type_id.to_i == 2
        {penalty_points: @penalties[:review]}
        pen.update_attribute(:penalty_points, @penalties[:review])
      elsif pen.deadline_type_id.to_i == 5
        {penalty_points: @penalties[:meta_review]}
        pen.update_attribute(:penalty_points, @penalties[:meta_review])
      end
    end
  end

T3: Modify the PenaltyHelper

The helper function penalty_helper.rb aides in calculating associated penalties with a late submission. In the original implementation, penalties were not calculated and returned. Minor modifications were made in the penalty_helper.rb file to return the value of calculated penalty.

T4: Modify the GradesController

The controller grades_controller.rb performs all the necessary functions involved with calculating scores. In the original implementation, the calculated penalties were not subtracted from the total score when a late submission was made. This controller is now modified such that based on the time of submission (if late) penalty points would be deducted from the total obtained score.

T5: Modify the ViewScores

When a student clicks on view scores link for an assignment he/she can view the scores obtained for that assignment. In the original implementation, the students could not view the penalty deductions obtained for the assignment. To enable this, additional code was added to /app/views/grades/_participant.html.erb, /app/views/grades/_participant_charts.html.erb and /app/views/grades/_participant_title.html.erb. Now when the student clicks on the view scores link, he/she can see all the penalty deductions in addition to the total score obtained.

T6: Test the working of the LatePolicy model

While creating a late policy many constraints have to be kept in mind like the policy name should not be blank or empty, every late policy should have an instructor associated with it, the maximum penalty and penalty per unit should be a positive number and the penalty unit should always be set (seconds/minutes/hours). We have ensured that these constraints function effectively by writing the necessary rspec tests. The rspec test can be located in /spec/models/late_policy_spec.rb.

late_policy_spec.rb

require 'rails_helper'

describe 'LatePolicy' do
  let (:late_policy) {LatePolicy.new policy_name: "Test", instructor_id: 1234, max_penalty: 20, penalty_per_unit: 2, penalty_unit: "hours"}

  describe "#policy_name" do
    it "returns the name of late policy" do
      expect(late_policy.policy_name).to eq("Test")
    end
    it "validates that the policy_name is not blank" do
      late_policy.policy_name = ' '
      expect(late_policy).not_to be_valid
    end
  end
  
  describe "#instructor_id" do
    it "returns the instructor id" do
    expect(late_policy.instructor_id).to eq(1234)
    end
  end

  describe "#max_penalty" do
    it "returns the maximum penalty value" do
      expect(late_policy.max_penalty).to eq(20)
    end
    it "validate max penalty is a number" do
      expect(late_policy.max_penalty).to eq(20)
      late_policy.max_penalty = 'a'
      expect(late_policy).not_to be_valid
    end
    it "should be greater than 0" do
      expect(late_policy.max_penalty).to eq(20)
      late_policy.max_penalty = -20
      expect(late_policy).not_to be_valid
      late_policy.max_penalty = 20
    end
    it "should be less than 0" do
      expect(late_policy.max_penalty).to eq(20)
      late_policy.max_penalty = 51
      expect(late_policy).not_to be_valid
      late_policy.max_penalty = 20
    end
  end

  describe "#penalty_per_unit" do
    it "returns the penalty per unit value" do
      expect(late_policy.penalty_per_unit).to eq(2)
    end
    it "validate penalty per unit is a number" do
      expect(late_policy.penalty_per_unit).to eq(2)
      late_policy.penalty_per_unit = 'a'
      expect(late_policy).not_to be_valid
    end
    it "should be greater than 0" do
      expect(late_policy.penalty_per_unit).to eq(2)
      late_policy.penalty_per_unit = -2
      expect(late_policy).not_to be_valid
      late_policy.penalty_per_unit = 2
    end
  end

  describe "#penalty_unit" do
    it "returns the penalty unit value" do
      expect(late_policy.penalty_unit).to eq("hours")
    end
    it "validates that the penalty_unit is not blank" do
      late_policy.penalty_unit = ' '
      expect(late_policy).not_to be_valid
    end
  end

end

Testing

Rspec Testing

As mentioned earlier, rspec tests are written to test the proper functioning of the LatePolicy model. The rspec tests can be found in /spec/models/late_policy_spec.rb file.

UI Testing

Testing LatePoliciesController

The create and the update code was refactored and modularized in the LatePoliciesController. The following steps can be followed from the Web UI to ensure that the refactoring did not affect the functionality of LatePoliciesController.

1. Login as instructor
2. Create an assignment in course
3. In the due dates tab in create assignment check the "Apply Late Policy" box
4. Select "New Late Policy" link
5. Fill in the form with necessary details and submit
6. The newly created late policy will be displayed in the list of all late policies
7. Select the "Edit" option for the created late policy
8. Change the required fields and save the changes
9. The updated late policy is displayed in the list of late policies
10. Edit the assignment/create a new assignment
11. In the due dates tab, the newly created late policy will be displayed in the dropdown

Testing Penalty Calculation and Deduction

Changes were made in the Penalty Helper file and Grades Controller to calculate the correct penalty value and deduct it from the total scores obtained. Changes were made in so that the View Scores option would display the penalties in addition to the total score. The following steps can be followed to ensure that the penalties get deducted appropriately from the total score and are visible to the student.

1. Login as instructor
2. Create an assignment in course along with a late policy
3. Create a team of 2-3 students for the created assignment
4. Login as a student
5. Make a submission for the assignment past the mentioned deadline
6. Go to view scores and the deducted penalty will be displayed

Link for demo

The demo for testing this functionality can be viewed at https://youtu.be/mOoIKkyu5cA. Please do turn on closed captions while viewing the video to obtain a better understanding.

Scope for future

While the late penalty has been implemented, there are few aspects of this project that can be improved.

1. Currently the review penalty is not being calculated because submission of review is not allowed after the mentioned deadline. If the review for an assignment is submitted after the mentioned deadline no points get deducted even after enabling late review. This will require code changes in assignment and other controllers.