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

From Expertiza_Wiki
Jump to navigation Jump to search
No edit summary
No edit summary
Line 65: Line 65:
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. <br/>
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. <br/>
To test this controller, run the following commands to verify that the code is working properly. <br/>
To test this controller, run the following commands to verify that the code is working properly. <br/>
=== Edge Cases ===
<pre>
<pre>
rspec spec/controllers/late_policies_controller_spec.rb
rspec spec/controllers/late_policies_controller_spec.rb
</pre>
</pre>
=== Edge Cases ===


====Screenshot of RSpec Test Cases Running Successfully====
====Screenshot of RSpec Test Cases Running Successfully====

Revision as of 20:12, 31 October 2022

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 to be addressed

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 if possible.
NOTE: I6 is already addressed, as both create and update methods have minimal usage of instance variables. The instance variables in the create method are used by the other parts of the program and hence cannot be replaced.
I7 : Add RSpec tests (Task completed, as the controller's CRUD operations along with the methods are being tested by the existing test cases even after the refactoring)

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. Update the code so that initially you assume that the penalty is NOT VALID, and then you need to verify that it is valid.




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

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

Edge Cases

Screenshot of RSpec Test Cases Running Successfully


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.

Screenshot of Deployed UI Running




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