CSC/ECE 517 Spring 2023 - E2327. Fix Back link on New Late Policy page

From Expertiza_Wiki
Jump to navigation Jump to search


Problem Description

Project Overview

Expertiza was created as an online platform to assist in peer reviewing and submission of academic work, which can include articles, codes, and websites. With this application, instructors are given the ability to add assignments, grade them, and allocate students into teams based on their preferred topics. The Expertiza platform provides a 'Late Policy' feature that can be applied to any assignment, so that points are automatically deducted if a student submits or reviews projects or assignments late after the due date has passed.The Late Policy feature is implemented by applying the policy to an assignment through the Due Dates tab during creation or editing of an assignment. The goal of this project is to address certain comments and issues associated with the solutions to the problems related to "Back" link on the "New Late Policy" page by making appropriate modifications and adding comprehensive test cases that cover all possible scenarios.

Project Scope

Aim of this Project

Te aim of this project is listed as follows:

  1. Improving code structure and documentation via refactoring
  2. Adding code comments for parts of code refactored or improved
  3. Improving code readability
  4. Writing tests for any changes made
  5. Removing redundant or unused code


Problems Resolved in the Current Implementation

Work Flow Diagram

Issues

The following problems have been covered and corrected in the current implementation:

Issue 1: Removing Error Message

Under the “Due Date” tab when click on the "New late policy" link an error message shows up.

Issue 2: Fixing Back Link

When creating a late policy, the “back” link does not take the user back to editing the assignment.

Issue 3: Fixing Back Link upon creation

After creating a late policy, the “back” link does not take the user back to editing the assignment.

Resolution

The following are the screenshots after the issues were resolved within the current implementation:

  • Issue 1 - Resolution where the error message doesn't show up.



  • Issue 2 - Displaying back link takes back to late policies

  • Issue 3 - After creation of late policy, back link takes back to late policies


Design Principle

The aim of our project is to improve upon the existing implementation by addressing the main issue which is fixing the backlinks and the error message displayed in Late Due Policy, which is explained at detail in the coming sections.

We checked review comments provided for the last implementation and improved upon it. Hence we will not be updating existing design patterns, and instead will be introducing refactoring within the existing code, by identifying DRY codes wherever possible and apply KISS principle if necessary.
Refactoring is the process of improving the structure, design, and/or implementation of an existing codebase without changing its external behavior. It involves making small, incremental changes to the code to make it more maintainable, easier to understand, and more efficient.

Solution and Files Refactored

Files Refactored or Modified

  • assignment_form.rb
  • assignments_controller.rb
  • late_policy_spec.rb

Changes made

We made the following refactorings within the existing solution to improve code readability and maintain sound workflow.

The code in assignments_controller.rb that is displaying an error in New late policy & breaking the backlinks was fixed. The error was due to the parameter assignment_id not being present in session parameter list. It was added and made necessary changes wherever the assignment_id was being rendered. This fix is present within the current implementation.


For the scope of our project, we identified several code areas where the code can be DRIED or KISS principles can be applied.

  • We extracted the method that is responsible for maintaining a check on duplicate assignment name and directory path for a given course ID and applied DRY - KISS principles. To ensure that the create method is readable and responsible for carrying out only a single responsibility of creating the assignment, we have separated the checking responsibilities from the code:




  • To improve the correctness of the code and avoid any illegal modifications of error messages, we have change the access modifier for the attribute error messages:



  • To improve code readability in the assignment controller, we have separated the functionality of updating the assignments form parameters. The update logic is now stored within a separate function 'update_form_params_with_existing' that is called within the create method of the controller.




  • To improve the code readability, we have refactored the local variable names used within the assignment form as follows:




Testing

RSpec Unit Tests

We plan to provide RSpec Unit Tests within the Late Policy Spec page on top of the existing tests in the form of simple unit tests to ensure that our changes and modifications do not break the working logic of the existing implementation.

Tests

Sr No Test Description
1 Test the refactoring for the copy method within the assignment_form.rb
2 Test successful assignment creation and redirection
3 Test redirection with error when assignment creation is invalid
4 Test setting assignment id within session


  • To the test the refactored copy method of the assignment form, we have included a unit test to mock the creation of a new assignment and trigger the copy method of Assignment object. We then verify the various fields of the copied object to ensure correctness after code refactoring.
require 'rails_helper'

RSpec.describe Assignment, type: :model do
  describe '.copy' do
    let!(:user) { create(:user) }
    let!(:old_assignment) { create(:assignment) }
    
    it 'creates a copy of the assignment' do
      new_assignment_id = Assignment.copy(old_assignment.id, user)
      
      expect(new_assignment_id).not_to be_nil
      expect(old_assignment.name).not_to eq(new_assignment.name)
      expect(old_assignment.directory_path).not_to eq(new_assignment.directory_path)
      expect(old_assignment.copy_flag).not_to eq(new_assignment.copy_flag)
    end
  end
end

  • In order to test the changes made within the create controller we have to introduce the following tests:
  1. Successful creation of an assignment and redirection to edit page
  2. Setting the assigment id within the session, so that it can be used in to route the 'Back' functionality
  3. Redirecting with error when assignment creation is invalid
RSpec.describe AssignmentsController, type: :controller do
  describe '#create' do
    let(:course) { create(:course) }
    let(:valid_assignment_attributes) { attributes_for(:assignment, course_id: course.id) }
    let(:invalid_assignment_attributes) { attributes_for(:assignment, course_id: course.id, name: nil) }
    
    context 'when the "Create Assignment" button is clicked and the assignment is valid' do
      before do
        post :create, params: { button: 'Create Assignment', assignment_form: { assignment: valid_assignment_attributes } }
      end
      
      it 'creates a new assignment and redirects to the edit assignment page' do
        expect(response).to redirect_to(edit_assignment_path(Assignment.last))
        expect(flash[:success]).to eq('Assignment "Test Assignment" has been created successfully. ')
      end
      
      it 'sets the session assignment_id to the new assignment ID' do
        expect(session[:assignment_id]).to eq(Assignment.last.id)
      end
    end
    
    context 'when the "Create Assignment" button is clicked and the assignment is invalid' do
      before do
        post :create, params: { button: 'Create Assignment', assignment_form: { assignment: invalid_assignment_attributes } }
      end
      
      it 'does not create a new assignment and redirects back to the new assignment page with an error message' do
        expect(response).to redirect_to(new_assignment_path(private: 1))
        expect(flash[:error]).to include('Failed to create assignment.')
        expect(flash[:error]).to include('Name can\'t be blank')
      end
    end
    
    
    context 'when the "Save" button is clicked' do
      before do
        post :create, params: { button: 'Save', assignment_form: { assignment: valid_assignment_attributes } }
      end
      
      it 'renders the new assignment page with an undo link' do
        expect(response).to render_template(:new)
        expect(assigns(:undo_link)).to eq('Assignment "Test Assignment" has been created successfully. ')
      end
    end
  end
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.

Team

Mentor

Divyang Doshi(ddoshi2@ncsu.edu)

Team Members

Amisha Bipin Waghela (awaghel@ncsu.edu)
Sasank Marabattula (smaraba@ncsu.edu)
Srilekha Gudipati (sngudipa@ncsu.edu)

Reference

Github PR for E2327

Expertiza_wiki

Expertiza Documentation

Expertiza Github