|
|
| Line 5: |
Line 5: |
|
| |
|
| == Description about project == | | == Description about project == |
| This page is a description of Expertiza OSS project E2126 which refactoring the account_request_controller.rb file. This file allows Super Administrators to manage the approval process for new users. Our job was to refactor some of the longer methods in this file, make error messages more intuitive, and add a security captcha when requesting the page to make a new user. | | This page is a description of Expertiza OSS project E2170 which aims to adequately test several files regarding the ResponseMap model. Here is a description of what response maps do: |
| | *In Expertiza, response maps establish a relationship between a reviewer, a reviewee, and an object to be reviewed. The reviewer is an assignment_participant, the reviewee is an assignment_team, and the reviewed object is either an assignment or another response_map. Each (reviewer, reviewee) pair will have a separate response map. Every time a new review is performed (e.g, every round), a new Response object is created whose map_id is that response map. |
|
| |
|
| ''DISCLAIMER:
| |
| *Some of the issues handed to us were already fixed when we looked at them from a previous project, so this page is only documenting the ones that we worked on. It can be assumed that the other undocumented issues are already done.
| |
| *'''THIS PROJECT IS NOT ABOUT MAKING A NEW USER.''' The purpose of the captcha is to verify if the user CAN BE REDIRECTED to the request new user page. The actual act of requesting a new user will error out due to some complications with the internal system's mailing system which is out of the scope of our assignments for this project. If you attempt to create a user, you are testing functionality that we were not assigned to fix.
| |
| ''
| |
| === Team === | | === Team === |
|
| |
|
| Line 17: |
Line 14: |
| Abir Majumder (aamajumd) | | Abir Majumder (aamajumd) |
|
| |
|
| Joshua Myers (jamyers3)
| | Quinn Dibble (qdibble) |
| | |
| | Alex Carruth (agcarrut) |
|
| |
|
| === Files Involved === | | === Files Involved === |
| Line 36: |
Line 35: |
|
| |
|
| == Relevant Methods == | | == Relevant Methods == |
|
| |
| * action_allowed?
| |
| * create_approved_user
| |
| * create_requested_user_record
| |
|
| |
| === action_allowed? ===
| |
|
| |
| This method validates if the user is allowed to be redirected to the appropriate page. The task for this project is to add a captcha system before the user requests new. This happens when the user hits the "Request account" button on the login page. This project's repo already has the "recaptcha" gem installed. Future developers need to keep in mind that functionality of the captcha depends on the developer updating the site key and secret key of the Google ReCaptcha API. Installation instructions of the gem and integrating the recaptcha API key can be found [https://github.com/ambethia/recaptcha here].
| |
|
| |
| There was one change to the frontend to display the captcha at _login.html.erb.
| |
|
| |
| '''Front end change /app/views/_login.html.erb'''
| |
| <pre>
| |
| ...
| |
| ...
| |
| <% end %>
| |
| <p>
| |
|
| |
| </p>
| |
| <br/>
| |
| </div>
| |
| <%= form_tag({:controller => 'account_request', :role => "Instructor", :action => 'new'}, :method => "get") do %>
| |
| <div>
| |
| <%= recaptcha_tags %> <------ Displays reCaptcha widget
| |
| </div>
| |
| <%= submit_tag "Request account", :class => 'btn btn-danger', :style => 'width:100%; margin-left:auto; margin-right:auto;' %>
| |
|
| |
| <% end %>
| |
| <% end %>
| |
| </pre>
| |
|
| |
|
|
| |
|
| '''action_allowed? changes'''
| |
| <pre>
| |
| def action_allowed?
| |
| case params[:action]
| |
| when 'list_pending_requested'
| |
| current_user_has_admin_privileges?
| |
| when 'new'
| |
| verify_recaptcha <---- function provided by recaptcha gem; validates frontend recaptcha input
| |
| when 'create_requested_user_record'
| |
| true
| |
| when 'keys'
| |
| current_user_has_student_privileges?
| |
| else
| |
| current_user_has_ta_privileges?
| |
| end
| |
| end
| |
| </pre>
| |
|
| |
| === create_approved_user ===
| |
|
| |
| This method allows a Super Admin to approve or reject new users, and gives them an error message if the incorrect steps are taken during that process.
| |
| <br>
| |
| ====Login info:====
| |
| Username: instructor6 Password: password
| |
| <br>
| |
| There has been a database change where the role of instructor6 was bumped up to Super Admin.
| |
|
| |
| ==== Original Method ====
| |
| <pre>
| |
| def create_approved_user
| |
| if params[:selection] == nil
| |
| flash[:error] = "Please Approve or Reject before submitting"
| |
| redirect_to action: 'list_pending_requested'
| |
| return
| |
| end
| |
| is_approved = (params[:commit] == "Accept"? "Approved" : "Rejected")
| |
| users = params[:selection]
| |
| users.each do |user|
| |
| requested_user = AccountRequest.find_by(id: user.first)
| |
| requested_user.status = is_approved
| |
| if requested_user.status.nil?
| |
| flash[:error] = "Please Approve or Reject before submitting"
| |
| elsif requested_user.update_attributes(params[:user])
| |
| flash[:success] = "The user \"#{requested_user.name}\" has been successfully updated."
| |
| end
| |
| if requested_user.status == "Approved"
| |
| new_user = User.new
| |
| new_user.name = requested_user.name
| |
| new_user.role_id = requested_user.role_id
| |
| new_user.institution_id = requested_user.institution_id
| |
| new_user.fullname = requested_user.fullname
| |
| new_user.email = requested_user.email
| |
| new_user.parent_id = session[:user].id
| |
| new_user.timezonepref = User.find_by(id: new_user.parent_id).timezonepref
| |
| if new_user.save
| |
| password = new_user.reset_password
| |
| # Mail is sent to the user with a new password
| |
| prepared_mail = MailerHelper.send_mail_to_user(new_user, "Your Expertiza account and password have been created.", "user_welcome", password)
| |
| prepared_mail.deliver_now
| |
| flash[:success] = "A new password has been sent to new user's e-mail address."
| |
| undo_link("The user \"#{requested_user.name}\" has been successfully created. ")
| |
| else
| |
| foreign
| |
| end
| |
| elsif requested_user.status == "Rejected"
| |
| # If the user request has been rejected, a flash message is shown and redirected to review page
| |
| if requested_user.update_columns(status: is_approved)
| |
| flash[:success] = "The user \"#{requested_user.name}\" has been Rejected."
| |
| # redirect_to action: 'list_pending_requested'
| |
| # return
| |
| else
| |
| flash[:error] = "Error processing request."
| |
| end
| |
| end
| |
| end
| |
| redirect_to action: 'list_pending_requested'
| |
| end
| |
| </pre>
| |
|
| |
| ==== Refactored Method ====
| |
|
| |
| This method was refactored so the creation of the new user is handled in a separate method called '''"user_new"'''
| |
|
| |
| <pre>
| |
| def create_approved_user
| |
| if params[:selection] == nil
| |
| flash[:error] = "Please Approve or Reject before submitting"
| |
| redirect_to action: 'list_pending_requested'
| |
| return
| |
| end
| |
| is_approved = (params[:commit] == "Accept"? "Approved" : "Rejected")
| |
| users = params[:selection]
| |
| users.each do |user|
| |
| requested_user = AccountRequest.find_by(id: user.first)
| |
| requested_user.status = is_approved
| |
| puts "Here"
| |
| if requested_user.status.nil?
| |
| flash[:error] = "Please Approve or Reject before submitting"
| |
| elsif requested_user.update_attributes(params[:user])
| |
| flash[:success] = "The user \"#{requested_user.name}\" has been successfully updated."
| |
| end
| |
| if requested_user.status == "Approved"
| |
| user_new(requested_user)
| |
| elsif requested_user.status == "Rejected"
| |
| # If the user request has been rejected, a flash message is shown and redirected to review page
| |
| if requested_user.update_columns(status: is_approved)
| |
| flash[:success] = "The user \"#{requested_user.name}\" has been Rejected."
| |
| # redirect_to action: 'list_pending_requested'
| |
| # return
| |
| else
| |
| flash[:error] = "Error processing request."
| |
| end
| |
| end
| |
| end
| |
| redirect_to action: 'list_pending_requested'
| |
| end
| |
|
| |
| def user_new(requested_user)
| |
| puts requested_user.inspect
| |
| new_user = User.new
| |
| new_user.name = requested_user.name
| |
| new_user.role_id = requested_user.role_id
| |
| new_user.institution_id = requested_user.institution_id
| |
| new_user.fullname = requested_user.fullname
| |
| new_user.email = requested_user.email
| |
| new_user.parent_id = session[:user].id
| |
| new_user.timezonepref = User.find_by(id: new_user.parent_id).timezonepref
| |
| if new_user.save
| |
| password = new_user.reset_password
| |
| # Mail is sent to the user with a new password
| |
| prepared_mail = MailerHelper.send_mail_to_user(new_user, "Your Expertiza account and password have been created.", "user_welcome", password)
| |
| prepared_mail.deliver_now
| |
| flash[:success] = "A new password has been sent to new user's e-mail address."
| |
| undo_link("The user \"#{requested_user.name}\" has been successfully created. ")
| |
| else
| |
| foreign
| |
| end
| |
| end
| |
| </pre>
| |
|
| |
| === create_requested_user_record ===
| |
|
| |
| create_requested_user creates a new account request for a user (if they are not a duplicate user) so that it may be approved by a higher level user.
| |
|
| |
| ==== Original Method ====
| |
|
| |
| <pre>
| |
| def create_requested_user_record
| |
|
| |
| requested_user = AccountRequest.new(requested_user_params)
| |
| #An object is created with respect to AccountRequest model inorder to populate the users information when account is requested
| |
|
| |
| if params[:user][:institution_id].empty?
| |
| institution = Institution.find_or_create_by(name: params[:institution][:name])
| |
| requested_user.institution_id = institution.id
| |
| end
| |
| #If user enters others and adds a new institution, an institution id will be created with respect to the institution model.
| |
| #This institution_attribute will be added to the AccountRequest model under institution_id attribute!
| |
|
| |
| #
| |
| requested_user.status = 'Under Review'
| |
| #The status is by default 'Under Review' until the super admin approves or rejects
| |
|
| |
| user_existed = User.find_by(name: requested_user.name) or User.find_by(name: requested_user.email)
| |
| # default to instructor role
| |
| if requested_user.role_id == nil
| |
| requested_user.role_id = Role.where(:name => "Instructor")[0].id
| |
| end
| |
| requested_user_saved = requested_user.save
| |
| #Stores a boolean value with respect to whether the user data is saved or not
| |
|
| |
| if !user_existed and requested_user_saved
| |
| super_users = User.joins(:role).where('roles.name = ?', 'Super-Administrator')
| |
| super_users.each do |super_user|
| |
| prepared_mail = MailerHelper.send_mail_to_all_super_users(super_user, requested_user, 'New account Request')
| |
| prepared_mail.deliver
| |
| end
| |
| #Notifying an email to the administrator regarding the new user request!
| |
| ExpertizaLogger.info LoggerMessage.new(controller_name, requested_user.name, 'The account you are requesting has been created successfully.', request)
| |
| flash[:success] = "User signup for \"#{requested_user.name}\" has been successfully requested."
| |
| redirect_to '/instructions/home'
| |
| #Print out the acknowledgement message to the user and redirect to /instructors/home page when successful
| |
|
| |
| return
| |
| elsif user_existed
| |
| flash[:error] = "The account you are requesting has already existed in Expertiza."
| |
| #If the user account already exists, log error to the user
| |
| else
| |
| flash[:error] = requested_user.errors.full_messages.to_sentence
| |
| #If saving in the AccountRequests model has failed
| |
| end
| |
| ExpertizaLogger.error LoggerMessage.new(controller_name, requested_user.name, flash[:error], request)
| |
| redirect_to controller: 'account_request', action: 'new', role: 'Student'
| |
| #if the first if clause fails, redirect back to the account requests page!
| |
| end
| |
| </pre>
| |
|
| |
| ==== Refactored Method ====
| |
|
| |
| The new create_requested_user_record now only handles the different cases that can happen when a user requests an account (New user and request goes through, duplicate user and request doesn't go through, or some other error in the saving process. The new method save_requested_user creates the request object, validates it has all needed fields and tries to save it to the database. The new method notify_supers_new_request sends an email to the Super-Administrators that new user account request has been successfully created. Some variable names and error messages have been reworded to improve readability as well.
| |
|
| |
| <pre>
| |
| def create_requested_user_record
| |
| requested_user = AccountRequest.new(requested_user_params)
| |
| #An object is created with respect to AccountRequest model inorder to populate the users information when account is requested
| |
| user_exists = User.find_by(name: requested_user.name) or User.find_by(name: requested_user.email)
| |
| requested_user_saved = save_requested_user(requested_user, params)
| |
| #Stores a boolean value with respect to whether the user data is saved or not
| |
| if !user_exists and requested_user_saved
| |
| notify_supers_new_request(requested_user)
| |
| redirect_to '/instructions/home'
| |
| return
| |
| elsif user_exists
| |
| flash[:error] = "The account you are requesting already exists in Expertiza."
| |
| #If the user account already exists, log error to the user
| |
| else
| |
| flash[:error] = requested_user.errors.full_messages.to_sentence
| |
| #If saving in the AccountRequests model has failed
| |
| end
| |
| ExpertizaLogger.error LoggerMessage.new(controller_name, requested_user.name, flash[:error], request)
| |
| redirect_to controller: 'account_request', action: 'new', role: 'Student'
| |
| #if the first if clause fails, redirect back to the account requests page!
| |
| end
| |
|
| |
| def save_requested_user(requested_user, params)
| |
| if params[:user][:institution_id].empty?
| |
| institution = Institution.find_or_create_by(name: params[:institution][:name])
| |
| requested_user.institution_id = institution.id
| |
| end
| |
| #If user enters others and adds a new institution, an institution id will be created with respect to the institution model.
| |
| #This institution_attribute will be added to the AccountRequest model under institution_id attribute!
| |
| requested_user.status = 'Under Review'
| |
| #The status is by default 'Under Review' until the super admin approves or rejects
| |
| # default to instructor role
| |
| if requested_user.role_id == nil
| |
| requested_user.role_id = Role.where(:name => "Instructor")[0].id
| |
| end
| |
| return requested_user.save
| |
| end
| |
|
| |
| def notify_supers_new_request(requested_user)
| |
| super_users = User.joins(:role).where('roles.name = ?', 'Super-Administrator')
| |
| super_users.each do |super_user|
| |
| prepared_mail = MailerHelper.send_mail_to_all_super_users(super_user, requested_user, 'New account Request')
| |
| prepared_mail.deliver
| |
| end
| |
| #Notifying an email to the administrator regarding the new user request!
| |
| ExpertizaLogger.info LoggerMessage.new(controller_name, requested_user.name, 'The account you are requesting has been created successfully.', request)
| |
| flash[:success] = "User signup for \"#{requested_user.name}\" has been successfully requested."
| |
| #Print out the acknowledgement message to the user and redirect to /instructors/home page when successful
| |
| end
| |
| </pre>
| |
|
| |
|
| == Results == | | == Results == |
|
| |
|
| 12 out of 12 tests in the account_request_controller_spec.rb test file.
| |
|
| |
| Here is a image of our tests passing:<br>
| |
| [[File:Passing Test.jpg]]
| |
|
| |
|
| === Relevant Links === | | === Relevant Links === |
|
| |
| Main Expertiza Repository can be found [https://github.com/expertiza/expertiza here].
| |
|
| |
| Our forked Repository can be found [https://github.com/majuabir/expertiza/tree/beta here].
| |
|
| |
| Our pull request to the beta branch can be found [https://github.com/expertiza/expertiza/pull/2117 here].
| |