CSC/ECE 517 Fall 2016/OSS E1637: Difference between revisions
No edit summary |
No edit summary |
||
Line 8: | Line 8: | ||
=== Controllers === | === Controllers === | ||
==== LatePolicyController ==== | ==== 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 | 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 ==== | ==== 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. | 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. | ||
Line 26: | Line 26: | ||
*T1: Refactor the create function in LatePoliciesController to make the code more modular | *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 | *T2: Refactor the update function in LatePoliciesController to make the code more modular | ||
*T3: | *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 | *T4: Modify the GradesController to deduct the total penalty from the total score | ||
*T5: Modify the ViewScores | *T5: Modify the ViewScores page to display the penalties deducted | ||
*T6: Test the working of the LatePolicy model by writing necessary RSpec tests | *T6: Test the working of the LatePolicy model by writing necessary RSpec tests | ||
Line 89: | Line 89: | ||
end | end | ||
end | end | ||
</pre> | |||
=== 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 ==== | |||
<pre> | |||
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 | |||
</pre> | </pre> | ||
== Testing == | == 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 === | === UI Testing === | ||
==== Testing LatePoliciesController ==== | ==== Testing LatePoliciesController ==== | ||
Line 115: | Line 200: | ||
=== Link for demo === | === Link for demo === | ||
The demo for testing this functionality can be | 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 == | == Scope for future == | ||
While the late penalty has been implemented, there are few aspects of this project that can be improved. | While the late penalty has been implemented, there are few aspects of this project that can be improved. | ||
#Currently the review penalty is not being calculated because | #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. |
Revision as of 05:54, 5 November 2016
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.
- Login as instructor
- Create an assignment in course
- In the due dates tab in create assignment check the "Apply Late Policy" box
- Select "New Late Policy" link
- Fill in the form with necessary details and submit
- The newly created late policy will be displayed in the list of all late policies
- Select the "Edit" option for the created late policy
- Change the required fields and save the changes
- The updated late policy is displayed in the list of late policies
- Edit the assignment/create a new assignment
- 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.
- Login as instructor
- Create an assignment in course along with a late policy
- Create a team of 2-3 students for the created assignment
- Login as a student
- Make a submission for the assignment past the mentioned deadline
- 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.
- 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.