CSC/ECE 517 Spring 2016/Refactor user course participant and assignment participant model: Difference between revisions

From Expertiza_Wiki
Jump to navigation Jump to search
Line 28: Line 28:
==Changes==
==Changes==


===Use SQL summation to calculate average_question_score===
===Refactoring: Use SQL summation to calculate average_question_score===
The average of the score for a question in the review given by all other students who reviewed an assignment needs to be calculated in the average_question_score method in assignment participant model. This was originally implement by using two for loops and fetching all the answers given by all the reviewers and then filtering using a if statement. This method consumes more CPU cycles and impacts the performance of the application server. It also consumes more bandwidth since the application fetches all the data from the database but returns only one value to the user. This was reimplemented to use the SQL summation inside the database and returns only value to the application. This method also takes advantage of the database indexing and fetches only values required instead of filtering in the application.
The average of the score for a question in the review given by all other students who reviewed an assignment needs to be calculated in the average_question_score method in assignment participant model. This was originally implement by using two for loops and fetching all the answers given by all the reviewers and then filtering using a if statement. This method consumes more CPU cycles and impacts the performance of the application server. It also consumes more bandwidth since the application fetches all the data from the database but returns only one value to the user. This was reimplemented to use the SQL summation inside the database and returns only value to the application. This method also takes advantage of the database indexing and fetches only values required instead of filtering in the application.
===Refactoring: Add attr_accessible to class to improve security===
The course_participant model has a number of instance variables like can_submit, can_review, user_id, parent_id, submitted_at, permission_granted, penalty_accumulated, grade, type, handle, time_stamp, digital_signature, duty, can_take_quiz. Earlier, these variables were less secure and hence susceptible to tampering with URLs or forms from malicious users. In general, elements are filled based on the data sent as a hash from the view. This happens as mass assignment where any matching key in the hash will be set. To protect this mass assignment from malicious attacks, a good design is to add attr_accessible macro. Adding this keyword to instance variables ensures that only the specified attributes in the list of attr_accessible are allowed for mass assignment. This ensures that developers can carefully consider which attributes should be allowed to do mass assignment. This way, the refactoring is done.
<code>attr_accessible :can_submit, :can_review, :user_id, :parent_id, :submitted_at, :permission_granted, :penalty_accumulated, :grade, :type, :handle, :time_stamp, :digital_signature, :duty, :can_take_quiz</code>
===Refactoring: files method is duplicate and used in assignment_team.rb as well. Move it to either a common place or keep it at the most appropriate place===
This refactoring belongs to AssignmentParticipant model. This model is a special class of Participant and it denotes the participants who have collaborated for a particular Assignment. This method takes a parameter which is a directory and it returns all the files present in that directory. The directory argument is basically a string containing the file path for reviewer uploaded files during peer review. This argument comes from AssignmentParticipant model but the files method as a whole is independent of the model as its function is to return files present in a directory. It does not need to belong to any model. Hence the ‘files’ method must be placed in file_helper.rb. Thus the ‘files’ method is moved from the AssignmentParticipant model to a helper module like file_helper.rb. The design principle that is reinforced here is that a class must have responsibilities that belong to itself and not other “helper” responsibilities.
<code>
  def files(directory)
    files_list = Dir[directory + "/*"]
    files = Array.new
    files_list.each do |file|
      if File.directory?(file)
        dir_files = files(file)
        dir_files.each{|f| files << f}
      end
      files << file
    end
    files
  end
</code>

Revision as of 00:34, 24 March 2016

E1603. Refactor user, course_participant and assignment_participant model

This page provides details about the changes made to Expertiza under project E1603. Refactor user, course_participant and assignment_participant model.

Background

Expertiza is an educational web application built on top of Ruby on Rails. The project is open source with code available on Github and maintained by both the students and faculty of the NC State University. The application allows instructors to manage courses and assignments. The students can choose a topic from the list for an assignment, form a team, submit their work and do multiple reviews on other's work. The application helps instructors and teaching assistants in scoring and maintaining the grades for the assignment.

Description

The users of the expertiza are modeled using the user model. A participant signs up for courses and assignments. A user can have multiple participants and all participants belong to a user. The participant model is inherited by the course and assignment participant model. Our work involves refactoring functions defined in these models. Some methods were moved to places with better fit and some methods were improvised with better implementations. The further sections in the document will explain the changes complement and how it will impact the overall usage of expertiza.

Tasks accomplished in this project

  • Moved the export_fields method to the User model from CourseParticipant model to remove redudancy.
  • Added attr_accessible modifier to CourseParticipant to improve security.
  • Refactored the get_user_list method into smaller methods, one method for each if condition in User model.
  • Moved the ‘files’ method from AssignmentParticipant model which was duplicate and used in assignment_team.rb as well to a common place (FileHelper module).
  • Refactored the average_question_score method by using sql summation to find values for sum_of_scores and number_of_scores variables in AssignmentParticipant model.

Models

User

The user model stores details of the user like name,email,role,timezone and his preferences on how expertiza should behave to him. A user can have many roles like student, instructor or administrator. The user model is used for logging in, participating in courses, assignments and creating teams. The user model maintains parent child relationship, a user will have one parent and can have many children.

Assignment Participant

Expertiza uses an participant abstraction for modeling the participance of a user in any of the activities like courses and assignments. A participant belongs a user and it represents a connection between user and a activity. Assignment_participant, which is a child class of participant models the association between users and assignments. This association also needs to be mapped to other activities performed by a user related to the assignment. These activities include reviews, quiz, response to reviews, teammate reviews etc.. The assignment_participant model is related to models which is responsible for all the mentioned activities.

Course Participant

The course_participant is a child class of the participant abstraction and it represents the association between a user and a course. The model also stores fields required for the association and supports bulk importing and exporting.

Changes

Refactoring: Use SQL summation to calculate average_question_score

The average of the score for a question in the review given by all other students who reviewed an assignment needs to be calculated in the average_question_score method in assignment participant model. This was originally implement by using two for loops and fetching all the answers given by all the reviewers and then filtering using a if statement. This method consumes more CPU cycles and impacts the performance of the application server. It also consumes more bandwidth since the application fetches all the data from the database but returns only one value to the user. This was reimplemented to use the SQL summation inside the database and returns only value to the application. This method also takes advantage of the database indexing and fetches only values required instead of filtering in the application.

Refactoring: Add attr_accessible to class to improve security

The course_participant model has a number of instance variables like can_submit, can_review, user_id, parent_id, submitted_at, permission_granted, penalty_accumulated, grade, type, handle, time_stamp, digital_signature, duty, can_take_quiz. Earlier, these variables were less secure and hence susceptible to tampering with URLs or forms from malicious users. In general, elements are filled based on the data sent as a hash from the view. This happens as mass assignment where any matching key in the hash will be set. To protect this mass assignment from malicious attacks, a good design is to add attr_accessible macro. Adding this keyword to instance variables ensures that only the specified attributes in the list of attr_accessible are allowed for mass assignment. This ensures that developers can carefully consider which attributes should be allowed to do mass assignment. This way, the refactoring is done. attr_accessible :can_submit, :can_review, :user_id, :parent_id, :submitted_at, :permission_granted, :penalty_accumulated, :grade, :type, :handle, :time_stamp, :digital_signature, :duty, :can_take_quiz


Refactoring: files method is duplicate and used in assignment_team.rb as well. Move it to either a common place or keep it at the most appropriate place

This refactoring belongs to AssignmentParticipant model. This model is a special class of Participant and it denotes the participants who have collaborated for a particular Assignment. This method takes a parameter which is a directory and it returns all the files present in that directory. The directory argument is basically a string containing the file path for reviewer uploaded files during peer review. This argument comes from AssignmentParticipant model but the files method as a whole is independent of the model as its function is to return files present in a directory. It does not need to belong to any model. Hence the ‘files’ method must be placed in file_helper.rb. Thus the ‘files’ method is moved from the AssignmentParticipant model to a helper module like file_helper.rb. The design principle that is reinforced here is that a class must have responsibilities that belong to itself and not other “helper” responsibilities.

 def files(directory)
   files_list = Dir[directory + "/*"]
   files = Array.new
   files_list.each do |file|
     if File.directory?(file)
       dir_files = files(file)
       dir_files.each{|f| files << f}
     end
     files << file
   end
   files
 end