CSC/ECE 517 Fall 2022 - E2252. Refactor auth controller.rb & password retrieval controller.rb
Overview of Expertiza
Expertiza is an open source software written using Ruby on Rails which functions as a learning management software system. It has man different functions and abilities including the ability to create assignments, quizzes, assignment groups and topics, and also a complete mechanism for providing peer reviews and feedback for other groups and other teammates. Part of its functionality is a system for user authentication with different user roles and permissions that determine how each user interacts with the content. The auth_controller.rb and password_retrieval_controller.rb which are the files primarily addressed in this project are both critical controllers in providing this functionality.
Description of Project
TODO: Describe the objectives of the project and what issues we were attempting to address.
auth_controller is used for authentication purposes.
The password_retrieval_controller deals with the process of updating and resetting a user password. The send_password method generates a token and appends it to a password reset URL. If a user submits a valid email address on the password_retrieval/forgotten view, the URL is sent to the user's email. When a user goes to the password reset URL, the token parameter is decrypted and checked for expiration. Next, the password_retrieval/reset_password view is loaded where a user enters an updated password and is sent back to the home page. In this project, the method was refactored in the following ways: to adhere to DRY principles, removal of hardwired constants, renaming of methods and variables, and enhanced comments. In addition, RSpec testing coverage of the controller was improved from 63.33% to 91.8% through a series of new tests that primarily look at the update_password method.
Files Modified
Changes to app/controllers/password_retrieval_controller.rb
| # | Change | Rationale | Commit Link |
|---|---|---|---|
| 1 | Updated check_reset_url method name to check_token_validity
|
The method validates that the password reset token is valid and present. The updated method name provides a more functionally descriptive name. | Commit |
| 2 | Replaced repeated code in lines 35-36 and 62-63 | The use of repeated code violates the DRY principle and so it was moved to a new method. | Commit |
| 3 | Change token expiration time to constant in line 41 | This time should not be hardwired; it should be a constant or a parameter. | Commit |
| 4 | Reload page if email is nil or empty on password_retrieval/forgotten view
|
An empty email parameter was causing the send password button to freeze. | Commit |
| 5 | Improve overall comments and rewrite error messages | The comments and error messages in the controller need to be more meaningful, specific and clear. | Commit |
Changes to spec/controllers/password_retrieval_controller_spec.rb
| # | Change | Rationale | Commit Link |
|---|---|---|---|
| 1 | Added two new RSpec tests for the update_password method
|
There were no tests for the update_password method. We wanted to enhance the test suite of this controller by increasing the coverage of its Rspec tests.
|
Commit |
| 2 | Added two new RSpec tests for the send_password method to check nil or blank input for email
|
There were no tests for the send_password method pertaining to checking invalid inputs in the request params
|
Commit Commit |
Changes to config/routes.rb
| # | Change | Rationale | Commit Link |
|---|---|---|---|
| 1 | Updated URL path and controller action to updated method name check_token_validity
|
The action and URL path must be renamed to generate pathing to the controller method and views. | Commit |
Changes to app/controllers/auth_controller.rb
| # | Change | Rationale | Commit Link |
|---|---|---|---|
| 1 | Move logger messages to before_action blocks wherever possible
|
Logger messages are inserted to log important events occurring in the code and do not relate directly to the logic. When possible, moving them to either before_action or after_action blocks makes the code more readable and easier to understand. It also separates the functionality of the method itself and the logging functionality.
|
Commit |
| 2 | Replaced repeated code for re-caching the user role | We noticed that although not listed on the recommended changes, this action involved exactly repeated code in the controller. The use of repeated code violates the DRY principle and so it was moved to a new method called self.rebuild_role_cache.
|
Commit |
| 3 | Improved helper function names | Originally we made the new helper functions used in logging have unhelpful, confusing names. Making them more clear helps the code to be more understandable. | Commit |
Testing
TODO: Show how the existing testing suite was passing before and after our refactoring - preserving functionality.
Testing password_retrieval_controller.rb
Before any refactoring to auth_controller.rb was done, we ran the rspec tests created for the controller with the following command: rspec spec/controllers/password_retrieval_controller_spec.rb
Tests prior to the changes covered 63.3% of password_retrieval_controller.rb.
After adding tests, the tests covered 91.1% of password_retrieval_controller.rb.
Testing auth_controller_.rb
Before any refactoring to auth_controller.rb was done, we ran the rspec tests created for the controller with the following command: rspec spec/controllers/auth_controller_spec.rb
After making all of the above changes to auth_controller.rb, we ran the rspec tests for the controller again with the command: rspec spec/controllers/auth_controller_spec.rb
We have successfully preserved the passing tests after the improvements we made to the auth_controller.rb.
Relevant Links
- Github Repository: https://github.com/greyfiles/expertiza
- Pull Request: https://github.com/expertiza/expertiza/pull/2460
- VCL Server: http://152.7.98.115:8080/
Contributors to this project
- Grey Files (unityid: mgfiles, github: greyfiles)
- Colin Odowd (unityid: cdodowd, github: colin-odowd)
- Pradyumna Khawas (unityid: ppkhawas, github: therealppk)