CSC/ECE 517 Fall 2019 - E1945. Refactor users controller.rb

About Expertiza

Expertiza is an open-source project based on Ruby on Rails framework. It allows the instructor to create new assignments and customize new or existing assignments. The instructor is allowed to create a list of topics for the students to which they can sign up for. For working on different projects and assignments the students can form teams in Expertiza. Peer review is another feature where students can review other students' submissions. This feature is available in Expertiza. Furthermore, Expertiza supports submission across various document types, including URLs and wiki pages.

Abstract

The UserController is a controller used for managing the creation, modification, and destruction of users in the Expertiza system. Instructor users may be added after creating a request for a new account. A key part of our team’s work was moving methods associated with managing a new account request from the UserController to a new controller named AccountRequestController. This removed coupling between account requests and user objects. Furthermore, it allowed an account request to be properly associated with its own controller, model, and view. Additionally, the team refactored and documented some methods in the UserController.

Problem Statement

The users_controller.rb file included the standard CRUD methods for a User model along with methods for other workflows. Most notably, the users_controller.rb file handled the creation and management of a RequestedUser object. This was problematic, as the Users controller should deal with User functionality, not requested user flows. The file also included a few methods which had a bad name or lack documentation. The functionality for paginating users did not work either, meaning that all users would be displayed on the List Users page. Thus changes were needed to make the code more readable, as well as to update certain views.

Our Work

Problem Solutions

The following tasks were required to be done for the project by our team according to the assignment:

• Separate all methods related to the workflow of a RequestedUser object
• Move below-mentioned methods to a new file named account_request_controller.rb

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

• The RequestedUser model should be renamed to AccountRequest and its lifetime must be managed by the new AccountRequestController
• The form that was currently displayed when the “Request Account” button is clicked from the Expertiza login page. It had to be edited with the following changes
  • Only instructor accounts can be created, so the dropdown had to be removed
  • All form labels had to be bold-faced
  • The “Self Introduction” label should be re-named to “Self-Introduction”
  • The textbox for the self-introduction field should include some hint (“Please include a link to your website”)
• Comments had to be written for the following methods

1. get_role
2. show_selection
3. foreign

• The paginate_list method had to be invoked at the correct location (in the list method) so that it paginates the users list correctly. Presently all users being shown on a single page by default on clicking the user list.

Changing the Request Account Page

This is the page that was loading originally when request account button is clicked in the login page:

Removing Dropdown

Only the Instructor account can be created and not TA which is there as an option originally. For this in the request_new.html.erb file, the selection tag has been changed to label tag in which the Instructor is put on the label

All form labels had to be bold-faced

For this in the individual forms inside the view are visited and bold tag has been added individually.

Renaming Self Introduction

Inside the _self_introduction.html.erb file the label is edited to the required one.

Adding hints in text-box

Initially, the text-box was blank and some hints had to be added. This was done by adding a place_holder attribute inside the text area inside the _self_introduction.html.erb file

This is the current page that gets loaded after fixing the above issues:

Adding comments for methods

The following methods had comments written or renamed for better understanding of the working of the methods and to reflect their actual behaviour.

1. get_role - The method was renamed to role, a comment was added explaining that it finds the role of a given user object
2. show_selection - The method was renamed to show_if_authorized, as this method should only display the users if the current user is authorized. Also changed it from a GET to a POST request since it more accurately reflects its working.
3. foreign - A comment was added for this method, explaining that it stores all roles possible and that it gets role id of the session’s user.

caption

Invoking paginate_list method

Invoked the paginate_list method at the correct location(in the list method) so that it paginates the users list correctly. The number of users per-page has currently been set to 100, which was showing all users in a single page by default. Also added a section at the bottom of the list.html.erb page for navigating between the paginated list of users.

As it can be seen now that there are page numbers and not all the users are being showed on the same page as was the case beforehand.

Results

The refactoring of the user_controller made the view for the account request more clearer and easy to understand for the user i.e. the UI was improved. Also, the code was made more cleaner and easy to read by for people who works on this later as the methods are segregated for performing their corresponding tasks and they comments are given for those which were not clear. Furthermore, the paginate users is now repaired and is working properly which lets the students list to be shown page wise instead of all the students in a single page which was not readable.

Video of Account Request Process:
https://youtu.be/Y0GIUZPQw3M

Test Plan

Tests for UserController were updated to account for the fact that some methods now use the AccountRequestController. The tests for the methods that were moved to AccountRequestController were moved to a new spec in the file account_request_spec.rb. The team did this because it made sense to make a new spec for a new controller. Nevertheless, most test functionalities remained the same. The tests were also updated to not select the user role from the “User Role” dropdown. This was done because the dropdown was removed from the UI since only instructor accounts can be created.

RSpec:

   account_request_spec
       context 'request account feature'
         it 'works correctly'
       context 'on users#list_pending_requested page'
         it 'allows super-admin and admin to communicate with requesters by clicking email addresses'
       context 'when super-admin or admin rejects a requester'
         it 'displays \'Rejected\' as status'
       context 'when super-admin or admin accepts a requester'
         it 'displays \'Accept\' as status and sends an email with randomly-generated password to the new user'
       context 'using name as username and password in the email'
         it 'allows the new user to login Expertiza'

   user_controller_spec
       context '#index'
         it 'redirects if user is student'
         it 'renders list if user is instructor'
       context '#set_anonymized_view'
         it 'redirects to back'
       context "#show_if_authorized"
         it 'user is nil'
         it 'user is not nil and user is available for editing'
         it 'user is not nil but is not available for editing'
       context '#show'
         it 'when params[:id] is not nil'
         it 'when params[:id] is not nil but role_id is nil'
         it 'when params[:id] is nil'
       context "#new"
         it '1'

Manual Testing

   AccountRequestController
     ApproveTest
       1. Navigate to Expertiza home page.
       2. Click Request User button.
       3. Fill in user information and submit.
       4. Login as super_administrator2 (password is "password").
       5. Click Administration -> Show -> Pending Requests
       6. Click Approve for the bottom-most request.
       7. Observe that the user's account request has been approved.

     RejectTest
       1. Repeat steps 1-5 from ApproveTest.Use different information for the user being created.
       2. Click Reject for the bottom-most request.
       3. Observe that the user's account request has been rejected.

       The above 2 tests check the flow of an AccountRequest's life cycle. They cover the refactored methods moved from UsersController to AccountRequestController. These methods were related to the creation of an account request, rejecting/approving it, and displaying a list of requests. This test covers all of those, and makes sure that no behavior regressed.

   PaginateUsers
       1. Login as instructor6 (password is"password").
       2. Click Manage -> Users.
       3. Scroll to the bottom of the user list page and observe that it is paginated.
       The above test checks that the list of users is now paginated. This is necessary to ensure that the paginate functionality was fixed and did not regress.

Useful Links

Github Repo: https://github.com/deepayanbardhan/expertiza
Pull Request: https://github.com/expertiza/expertiza/pull/1541

Team Information

Project Mentor:
Ramya Vijayakumar

Project Members:
Benjamin Fisher
Deepayan Bardhan
Sanket Pai