: Difference between revisions
No edit summary |
No edit summary |
||
Line 19: | Line 19: | ||
The assignments were identified using @assignment.errors.to_s to convert Active Model Errors object to string which failed to display the error message. We refactored it to @assignment.error.full_messages to help identify the error message. <br> | The assignments were identified using @assignment.errors.to_s to convert Active Model Errors object to string which failed to display the error message. We refactored it to @assignment.error.full_messages to help identify the error message. <br> | ||
On analyzing issue 1, we found that the error message appeared when validation checks for assignment_form model failed for one particular assignment named "Program 2". This particular assignment failed the validation of having the sum of rubric metric being either 0 or 100. Based on our understanding, this data was probably inserted prior to the above mentioned validation checks in app/model/assignments_form.rb file.<br> | On analyzing issue 1, we found that the error message appeared when validation checks for assignment_form model failed for one particular assignment named "Program 2". This particular assignment failed the validation of having the sum of rubric metric being either 0 or 100. Based on our understanding, this data was probably inserted prior to the above mentioned validation checks in app/model/assignments_form.rb file.<br> | ||
< | <br> | ||
'''Previous Code: ''' Using @assignment.errors.to_s to convert Active Model Errors object to string failed to display the error message. We refactored it to @assignment.error.full_messages to help identify the error messages. | '''Previous Code: ''' Using @assignment.errors.to_s to convert Active Model Errors object to string failed to display the error message. We refactored it to @assignment.error.full_messages to help identify the error messages. |
Revision as of 16:35, 12 December 2022
Problem Description
Expertiza can automatically deduct points if a student is late in performing some action (e.g., submitting or reviewing). This is implemented by defining a new “late policy” and applying it to the assignment. Late policies are managed on the Due dates tab of assignment creation (or editing). The goal of this project is to eliminate issues associated with the "Back" link on "New Late Policy" page by making necessary changes and adding new test cases which cover all scenarios.
Project Scope
Issues with the Current Implementation
The following issues have been raised in the current implementation:
- Issue 1 - Under the “Due Date” tab when click on the "New late policy" link an error message shows up.
- Issue 2 - When creating a late policy, the “back” link does not take the user back to editing the assignment.
- Issue 3 - After creating a late policy, the “back” link does not take the user back to editing the assignment.
Solution and Files Refactored
- Issue 1: Under the “Due Date” tab when click on the "New late policy" link an error message shows up.
1. app/models/assignment_form.rb
The assignments were identified using @assignment.errors.to_s to convert Active Model Errors object to string which failed to display the error message. We refactored it to @assignment.error.full_messages to help identify the error message.
On analyzing issue 1, we found that the error message appeared when validation checks for assignment_form model failed for one particular assignment named "Program 2". This particular assignment failed the validation of having the sum of rubric metric being either 0 or 100. Based on our understanding, this data was probably inserted prior to the above mentioned validation checks in app/model/assignments_form.rb file.
Previous Code: Using @assignment.errors.to_s to convert Active Model Errors object to string failed to display the error message. We refactored it to @assignment.error.full_messages to help identify the error messages.
def update_assignment(attributes) unless @assignment.update_attributes(attributes) @errors = @assignment.errors.to_s @has_errors = true end @assignment.num_review_of_reviews = @assignment.num_metareviews_allowed .....
Modified Code:
def update_assignment(attributes) unless @assignment.update_attributes(attributes) @errors = @assignment.errors.full_messages @has_errors = true end @assignment.num_review_of_reviews = @assignment.num_metareviews_allowed .....
Previous Code: Using @assignment.errors.to_s to convert Active Model Errors object to string failed to display the error message. We refactored it to @assignment.error.full_messages to help identify the error messages.
def update_assignment_questionnaires(attributes) if attributes[0].key?(:questionnaire_weight) validate_assignment_questionnaires_weights(attributes) @errors = @assignment.errors.to_s topic_id = nil end unless @has_errors ......
Modified Code:
def update_assignment_questionnaires(attributes) if attributes[0].key?(:questionnaire_weight) validate_assignment_questionnaires_weights(attributes) @errors = @assignment.errors.full_messages topic_id = nil end unless @has_errors ........
Previous Code: Using @assignment.errors.to_s to convert Active Model Errors object to string failed to display the error message. We refactored it to @assignment.error.full_messages to help identify the error messages.
def update_assignment_questionnaires(attributes) aq = assignment_questionnaire(questionnaire_type, attr[:used_in_round], topic_id, duty_id) if aq.id.nil? unless aq.save @errors = @assignment.errors.to_s @has_errors = true next end end unless aq.update_attributes(attr) @errors = @assignment.errors.to_s @has_errors = true end end
Modified Code:
def update_assignment_questionnaires(attributes) aq = assignment_questionnaire(questionnaire_type, attr[:used_in_round], topic_id, duty_id) if aq.id.nil? unless aq.save @errors = @assignment.errors.full_messages @has_errors = true next end end unless aq.update_attributes(attr) @errors = @assignment.errors.full_messages @has_errors = true end end
Previous Code: Using @assignment.errors.to_s to convert Active Model Errors object to string failed to display the error message. We refactored it to @assignment.error.full_messages to help identify the error messages.
def update_due_dates(attributes, user) # get deadline for review @has_errors = true unless dd.update_attributes(due_date) end @errors += @assignment.errors.to_s if @has_errors end end
Modified Code:
def update_due_dates(attributes, user) # get deadline for review @has_errors = true unless dd.update_attributes(due_date) end @errors += @assignment.errors.full_messages if @has_errors end end
- Issue 2 and Issue 3: When creating a late policy and after creating a late policy, the “back” link does not take the user back to editing the assignment
1. app/views/late_policies/new.html.erb
In order to resolve this issue we stored the assignment_id as parameter in the session variable in app/controllers/assignments_controller.rb
and then used this assignemnt_id in the Back link of index and new page of late policy to redirect back to edit view for assignment.
Previous Code:
<%= link_to 'Back', :controller=> 'assignments', :action => 'edit', :id=>session[:assignment]%>
Modified Code:
<%= link_to 'Back', :controller=> 'assignments', :action => 'edit', :id=>session[:assignment_id]%>
2. app/views/late_policies/index.html.erb
Previous Code:
<%= link_to 'New late policy', :action => 'new'%><br /> <%= link_to 'Back', :controller=> 'assignments', :action => 'edit', :id=>session[:assignment]%>
Modified Code:
<%= link_to 'New late policy', :action => 'new'%><br /> <%= link_to 'Back', :controller=> 'assignments', :action => 'edit', :id=>session[:assignment_id]%>
3. app/controllers/assignments_controller.rb
We stored the assignment_id as parameter in the session variable in app/controllers/assignments_controller.rb
Previous Code:
def create ...... assignment_form_params[:due_date] = due_array @assignment_form.update(assignment_form_params, current_user) aid = Assignment.find(@assignment_form.assignment.id).id ExpertizaLogger.info "Assignment created: #{@assignment_form.as_json}" redirect_to edit_assignment_path aid undo_link("Assignment \"#{@assignment_form.assignment.name}\" has been created successfully. ") return
# edits an assignment's deadlines and assigned rubrics def edit user_timezone_specified edit_params_setting assignment_staggered_deadline? ....... @badges = Badge.all @use_bookmark = @assignment.use_bookmark @duties = Duty.where(assignment_id: @assignment_form.assignment.id) end
# displays an assignment via ID def show @assignment = Assignment.find(params[:id]) end
Modified Code:
def create ...... assignment_form_params[:due_date] = due_array @assignment_form.update(assignment_form_params, current_user) assignment_id = Assignment.find(@assignment_form.assignment.id).id ExpertizaLogger.info "Assignment created: #{@assignment_form.as_json}" session[:assignment_id] = assignment_id redirect_to edit_assignment_path assignment_id undo_link("Assignment \"#{@assignment_form.assignment.name}\" has been created successfully. ") return
# edits an assignment's deadlines and assigned rubrics def edit session[:assignment_id] = nil user_timezone_specified edit_params_setting assignment_staggered_deadline? ...... @badges = Badge.all @use_bookmark = @assignment.use_bookmark @duties = Duty.where(assignment_id: @assignment_form.assignment.id) @assignment = Assignment.find(params[:id]) session[:assignment_id] = @assignment.id end
# displays an assignment via ID def show session[:assignment_id] = nil @assignment = Assignment.find(params[:id]) session[:assignment_id] = @assignment.id end
4. app/views/assignments/edit.html.erb
In app/views/assignments/edit.html.erb
we added a JQuery to ensure that if the user comes from the late_policy page, it will redirect them to the Due Dates tab of the edit assignment page.
Previous Code:
No code
Modified Code:
// Redirect to Due Date Tab on Back link from late policy jQuery(document).ready(function() { var referrer = document.referrer; if(referrer.includes("late_policies")){ $('#DueDates').click(); } });
Design Principle
Since our goal is to fix existing functionalities we will not be updating the existing design patterns being employed in the code.
Refactoring is a systematic process of improving code without creating new functionality. Thus, a key to the success of our project is ensuring everything that was working before our changes work even after our changes have been added. To ensure this, we will continue to test the system after each issue has been fixed. This will allow us to ensure two things:
- We are only changing what we set out to change when fixing a particular issue.
- We are not breaking what was working before we deployed our fix.
Testing
RSpec Unit Tests
We have written RSpec tests for verifying our code for the following use cases:
Case 1:
Scenario: The instructor is about to create a new late policy.
Given: Logged in as an instructor.
When: The instructor tries to create a new late policy by clicking 'New Late Policy' under the 'Due Dates' tab/
Then: The instructor is redirected to the create/edit policy page without any error message displayed. <be>
it 'Flash Error Message on New Late Policy Page', js: true do assignment = Assignment.where(name: 'assignment for late policy test').first create(:topic, assignment_id: assignment.id) visit "/assignments/#{assignment.id}/edit" click_link 'Due Dates' click_link 'New Late Policy' fails if expect(flash[:error]).to eq('Failed to save the assignment: ') end end
Case 2:
Scenario: The instructor is creating a new late policy and tries to go back to the previous page.
Given: Logged in as an instructor and in the 'New Late Policy' page
When: The instructor tries to go to the previous page
And: The instructor clicks the “Back” button.
Then: The instructor is redirected to the previous Due Dates page.
it 'Back Button Interation on New Late Policy Page', js: true do assignment = Assignment.where(name: 'assignment for late policy test').first create(:topic, assignment_id: assignment.id) visit "/assignments/#{assignment.id}/edit" click_link 'Due Dates' click_button 'New Late Policy' click_button 'Back' expect(page).to route_to("/assignments/#{assignment.id}/edit") end
Case 3:
Scenario: The instructor has created a new late policy and tries to go back to the previous page.
Given: Logged in as an instructor.
When: The instructor successfully creates a new late policy, being redirected to the “/late_policies” webpage with all information on late policies.
And: The instructor clicks the “Back” button.
Then: The instructor is redirected to the previous create/edit assignment page.
it 'Back Button Interation on New Late Policy Page while creating', js: true do assignment = Assignment.where(name: 'assignment for late policy test').first create(:topic, assignment_id: assignment.id) visit "/assignments/#{assignment.id}/edit" click_link 'Due Dates' click_button 'New Late Policy' fill_in "policy_name", with: 'Test Late Policy' fill_in "penalty_per_unit", with: '15' fill_in "max_penalty", with: '20' click_button 'Create' visit "/late_policies" click_button 'Back' expect(page).to route_to("/assignments/#{assignment.id}/edit") end
Manual Testing
- When logging in as an instructor:
1. Edit an assignment by clicking on edit logo under the “Action” column.
2. Under the “Due Date” tab click on the "New late policy" link.
3. In “New late policy” fill in the required details.
4. Clicks “Create” to save the policy, to go to page which shows all late policies.
5. Clicks “Back” to redirect the instructor back to the “Due Date” tab of the assignment which was being edited.
Or
4. Clicking “Back”, should redirect the instructor back to the “Due Date” tab of the assignment which was being edited.
Link to UI Testing Demo https://drive.google.com/file/d/1iYq2_Zf_Q-2UnHh3e4NGkJ742lJFMtzy/view?usp=sharing
Important Links
Github Links
Link to Expertiza repository: https://github.com/expertiza/expertiza
Link to the forked repository: https://github.com/NitishKumar2404/Expertiza
Program 4 code is present in program4 branch: https://github.com/NitishKumar2404/Expertiza/tree/program4
Link to Pull Request: https://github.com/expertiza/expertiza/pull/2482
Team
Mentor
Vinay Deshmukh (vdeshmu@ncsu.edu)
Team Members
Kalgee Anand Kotak (kkotak@ncsu.edu)
Ashrita Ramaswamy (aramasw@ncsu.edu)
Nitish Kumar Murali (nmurali2@ncsu.edu)