CSC/ECE 517 Fall 2022 - E2256: Refactor late policies controller.rb

From Expertiza_Wiki
Jump to navigation Jump to search

Background

The SCRUM from late policies is managed by the later policies controller. The file primarily determines whether newly added or updated late policies are valid policies by ensuring that they do not share the same name as an already existing policy. The file also ensures that the late penalty per unit of time is less than the maximum late penalty, and restricts late penalty to have a maximum value of 100.

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)

Description about project

This wiki describes the work done to refactor late policies controller.rb. The main goals of the effort were to eliminate redundant code and to make the create and update methods' code easier to read. This also includes adding code comments for understandability.

Files Modified

  • app/controllers/late_policies_controller.rb
  • app/views/late_policies/new.html.erb
  • app/views/late_policies/_form.html.erb
  • app/views/late_policies/show.html.erb
  • app/views/late_policies/edit.html.erb
  • app/views/late_policies/index.html.erb
  • spec/controllers/late_policies_controller_spec.rb

Issues Fixed

We worked on the following Issues (I#)
I1 : Rename all usages of @penalty_policy to @late_policy within the controller as well as the views
I2 : Refactor the method “duplicate_name_check” such that it doesn’t violate the Single Responsibility Principle. Use a better name than “duplicate_name_check”
I3 : Make the validate_input method as “stateless” as possible.
I4 : Update the code so that initially you assume that the penalty is NOT VALID, and then you need to verify that it is valid during validation.
I5 : Fix the Error Message output. It currently prints only one error message even if more than one validation is failing.
I6 : Use local variables instead of instance variables in the create and update method.
I7 : Add RSpec tests

Changes Implemented

  • Renaming @penalty_policy to @late_policy within controller and views
         Commit Link: https://github.com/NitishKumar2404/Expertiza/commit/61b01faf67bc903beb03e6d33d6abcc640e78d48
  • Refactor the method “duplicate_name_check” such that it doesn’t violate the Single Responsibility Principle. Use a better name than “duplicate_name_check”. Make the validate_input method as “stateless” as possible
         Pull Request Link: https://github.com/NitishKumar2404/Expertiza/pull/2
  • Update the code so that initially you assume that the penalty is NOT VALID, and then you need to verify that it is valid.
         Commit Link: https://github.com/expertiza/expertiza/pull/2466/commits/3af63f214d86b8008dc5db74195a5b77fb8aa4c6
  • While creating or updating a late policy, in cases where a user enters multiple invalid inputs, only the last error message was shown. Ideally, all error messages should be printed so that the user can fix all the errors before the next time they submit. We modified the code in late_policies_controller.rb to incorporate this change by concatenating the error message so that all errors are taken into account.
         Commit Link: https://github.com/NitishKumar2404/Expertiza/commit/f8c313e0efe71bceb3d39afb897728ea83ecae5e

Peer Review Information

For users intending to view the deployed Expertiza associated with this assignment, the credentials are below:

  • Instructor login: username -> instructor6, password -> password

Test Plan

RSpec

This is a refactoring project, so we tested using the existing test cases in late_policies_controller_spec.rb. Since all the test cases are passing it implies that the refactored code does not break any of the existing functionality. We have also added some new test cases in the rspec.
To test this controller, run the following commands to verify that the code is working properly.

rspec spec/controllers/late_policies_controller_spec.rb

UI Testing

The functionality can be tested manually by following steps.
1. Navigate over to the late policies index page: http://152.7.177.201:8080/late_policies.
2. Create a new policy using same name as existing policy and see if you get proper error message.
3. Try updating an existing policy by passing in the name that is already in use and see if you get a proper error message.
4. Try different input combinations and test if error messages appears for invalid input.


GitHub links

Link to Expertiza repository: https://github.com/expertiza/expertiza
Link to the forked repository: https://github.com/NitishKumar2404/Expertiza
Linked to Pull Request: https://github.com/expertiza/expertiza/pull/2466