CSC/ECE 517 Spring 2022 - S2222: Refactor impersonate controller

From Expertiza_Wiki
Revision as of 00:24, 22 March 2022 by Sskarra (talk | contribs)
Jump to navigation Jump to search

This wiki page describes the work done under E2222 OSS Program for Spring 2022, in the CSC/ECE 517 course.



About Expertiza

Expertiza is an open-source project based on Ruby on Rails framework. Expertiza is a complete instructor-student usage website where the instructor can assign assignments, deadlines, grades, etc that is required for the course(s) under him and the students can use this website to perform the tasks required as part of the course like project or assignments submission, forming groups and collaborating with them, as well as reviewing projects and teammates.

This project focuses on a specific feature of expertiza which allows Administrators, Instructors or Teaching Assistants to impersonate other and access their account. The demonstration for the feature is as shown below.

Figure 1
Figure 2
Figure 3

What does this controller do?

Login: We can only login as an instructor(only instructor login details are available) username -> instructor6, password -> password

  • When logged in as an instructor, under the manage option as shown in Figure 1, select impersonate user.
  • Upon redirected to impersonate page, enter the account which needs to be impersonated.
  • It impersonates that user provided that user can be impersonated.
  • As shown in figure 3, this can be used to revert the impersonation and return to the instructor profile.

Problem Statement

Expertiza allows administrators, instructors and Teaching Assistants to impersonate other users like a student. This allows the impersonator to view assignments, deadlines and submissions of other students. The rules to impersonating a user is, the impersonator has to be an ancestor of the impersonate. The hierarchy of impersonation is as follows:

super administrator -> Administrator -> Instructor -> Teaching Assistant -> Student

Note: impersonation cannot happen within the same level of hierarchy. For Example, a Super Administrator can impersonate any user apart from other Super Administrators, an Administrator can impersonate Instructors, TA, Students and not other Admins and so on. The aim of this project is to refactor the impersonate controller.

Major issues in the previous version of code

  • Long single lines of code throughout the controller.
  • Repeated code written multiple times in different methods throughout the controller
  • Method names used were not informative and readable.
  • Undiscovered bug that was breaking the application(Has been reported in Expertiza).
  • Unused method discovered.

What needs to be done

  • Introduce Single Responsibility Principle - A method/function should only have one purpose to fulfill (Read more about it here). Read more about the ideal length of a function here.
  • You’re supposed to figure out different things a method is doing, break that method into smaller methods such that each resulting method is doing only one thing.
  • Throughout the file, the user is being initialized but never used. This is bad coding practice as it slows down runtime.
  • Code comments are needed for each and every function. Figure out what the function is doing and write at-least 50 word descriptions for each function.
  • Your code comments should also include the information about what the parameters are for, and what the method returns (not counted for 50 word description).
  • Read more about how to write meaningful code comments here.
  • Rename the methods to a more informative name. For example, There can be better names for check_if_user_impersonateable method, do_main_operation, check_if_special_char methods
  • Refactor very long lines of code to make it more readable
  • There are many unnecessary nesting of if statements. Convert them to elif. For example, Line 36, Line 139.
  • Use a guard clause instead of wrapping the code inside a conditional expression. Read more about guard clauses here.
  • A generalized exception is being rescued everywhere, which is against the coding principles that are supposed to be followed. Find what exceptions could a piece of code generate and rescue a more specialized exception.
  • Test Functions and increase test code coverage.

Solution