E1914 Refactor users controller
This is an Expertiza based OSS Project.
Problems with the current implementation
1) The users_controller.rb file included the standard CRUD methods for a User model along with methods for other workflows. The users_controller.rb file handled the creation and management of a RequestedUser object.
2) The users_controller included a few methods which have a bad name or lack documentation.
3) The forms that come after "Request Account" button is clicked need to be changed.
4) When a list of all users are shown, the list is not paginated.
Solutions to the problems
Earlier, there was no controllers for a RequestedUser object and these were handled by the Users controller itself. We separated the RequestedUser workflow from the Users and we renamed the RequestedUser model to AccountRequest.
As a part of this refractoring activity, we created a new controller was created called the account_requests_controller to handle the workflow of an AccountRequest
The following methods were moved from users_controller to account_requests_controller.
Following methods were moved from the users_controller to account_requests_controller which was a part of the requirements.
1) created_approved_user
2) list_pending_requested
3) request_new
4) created_requested_user_record
5) roles_for_request_sign_up
6) requested_user_params
RSpec testing for account_requests_controller
It can be seen that all 11 test cases passed for account_request_controller.
RSpec testing for users_controller
It can be seen that all 18 test cases passed for users_controller.
Writing comments to make methods more understandable
Wrote comments for the 'foreign' method. The comments include:
1) What the method does: This method is used to find the list of roles the current user can embody.
2) Where the method is used: Used to display a drop-down selection of roles for the current user in the views.
Wrote comments for the show and show_selection methods:
1) What the method does:
show(): If the current user is a student, they should only be able to see information about themselves. All other people should be able to see information about themselves or other students. If the request to show() passes these checks, then they are shown the view 'show'. Otherwise, they are redirected to the home page.
show_selection(): If the role of a user's parent is less than the current user or if the current user is requesting to see itself or if user's parent_id does not exist then the show() method is called. All these conditions boil down to whether the current user of the system is authorized to see/edit the information about the user specified in the params. If the requested user does not exist or if the current user is not authorized to see the requested user, then the current user is redirected back to the list page.
2) Scenarios in which show_selection is called: in users/list.html.erb, a list of users is shown. On top of the list there is a functionality to search the list. When a person searches for a particular student, and selects that student, the show_selection method is called. If the person is allowed to see that student, the user is directed to the show() method. Otherwise the person stays on the list view.
3) Scenarios in which show() is called: From the edit.html.erb, if the person wants to see the information instead of editing it. From show_selection() as described above.
Renaming methods to reflect their actual behavior
The get_role method was named more like it would be in Java. It was renamed to 'role' because that is how it would be named in ruby. We chose to keep the method in the controller instead of moving it to the model because it does not alter the data structure and it also has selection logic.
Make GUI changes in the form displayed after "Request Account" button is clicked
Only instructor accounts can be created, so the drop-down was removed.
All form labels were bold-faced.
The “Self Introduction” label was re-named to “Self-Introduction”.
The text-box for the self-introduction field now includes a hint "Please include a website name".
Before it looked like this :
After it looks like this:
Paginating the list of users
The existing project does not have the feature of pagination for the user list. So, currently, all the users are shown to the instructor.
The paginate_list method is supposed to paginate the list of users. The method was not called anywhere and also the method logic was incorrect.
With the latest implementation, the pagination is added with dropdown option - "25", "50", "100" and "ALL"
The method "paginate_list" is corrected and called at right place
The default option is kept to "25" to reduce the web page loading time.
Before it looked like this:
After it looks like this:
Design choices
1. There are some common partials which are used in Users views and Account_requests views. Users and Account_requests are two different entities. So, the team along with our mentor has decided to keep the common partials in both folders so that project remains flexible with future incoming changes regarding these views. Redundant partials are email, name, institution, password, and prefs.
2. We refactored the variables names in requested_users to account_requests (in views, controller actions, etc.) since requested_users is a bad naming choice for our variable. Since we are referring to AccountRequest model in our controllers, such renaming makes sense here.
For example:
account_request = AccountRequest.all
makes more sense than
requested_users = AccountRequest.all