From Expertiza_Wiki
Revision as of 16:38, 12 December 2022 by Aramasw (talk | contribs)
Jump to navigation Jump to search

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.

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: 

  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
	.....

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: 

   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
	........

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: 

    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

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: 

    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)

Retrieved from ""