Expertiza_Wiki - User contributions [en]

CSC/ECE 517 Spring 2016 E1629 Performance improvement for Course & Assignment Listing

2016-04-11T22:01:46Z

Nkerkar: Use case diagrams added

== Purpose ==
The purpose of this project is to improve the performance of the Course and Assignment listing page. When an instructor or TA logs in to the system they are redirected to the course/assignment/questionnaire list page. This page lists all the courses that are created by the logged in user and also the public courses that are created by other users. The current implementation of the page is in ReactJS and takes a long time to load. The purpose of this project is to improve the performance of this functionality so that it takes lesser time to display the relevant data.

== Problem Statement ==
The course assignment listing page has three tabs. These tabs are '''Courses''', '''Assignments''' and '''Questionnaires'''. The courses tab lists all all the courses that the instructor/TA might be interested in. This includes all the courses that they created and also the public courses. Additionally in the courses page all the assignments related to each course is also loaded when the page is loaded. All assignments belonging to a course can be viewed by clicking on that course.
On the listing page there exists a check-box which allows the user to select whether or not to display public projects. By default this option is selected and as a result all public and private courses are displayed. Similarly for the assignments page all the public as well as private assignments created by the user are loaded. Because of this the page takes a long time to load. This project is concerned with coming up with solutions so that the page is loaded in a quicker manner. The individual work items identified are as follows:
* ''Include others' items'' check-box should be unchecked by default for both courses and assignments page. By default, the public courses should not be loaded.
* Current implementation of the Course page makes separate REST calls to the controller for fetching assignments for each course. This is a bad design and it takes a while to load the whole page. we plan to modify such that the assignments are loaded only when a particular course is clicked.
* List the courses in two different sections. One section should be to list all the courses that are created by the user. The other section should be to display all the public courses. This section should be displayed only if the check-box is selected. Searching for courses in the current single list implementation is difficult. Partitioning the list into two sections would greatly improve the usability of the page.
* Similarly the assignments tab should be divided into two sections. The first section should list all the assignments that are created by the user and the next should list all the public assignments.
=== Magnitude of Problem ===
In order to better understand the problem consider the following scenario:<br>When logged in as an instructor(instructor6) the course page loads a total of 103 courses. Hence it will make 103 REST calls to fetch assignments. Out of these 25 are concerned with the signed in instructor. Hence if we only load courses related to the instructor by default it drastically reduces the amount the work done. Also we will load assignment details only on demand.

== Overview of Approach ==
The page loads the details of the courses and assignments related to all the courses during the page load. But the assignment details are hidden are not shown for each course. They are shown only when a user clicks on a assignment. We can improve the speed of the page load by postponing the loading of assignments till the user clicks on a course. This will involve making an asynchronous call to the server and fetching the assignment details when a clicks on the course. We will also not load the public courses by default and give an option to load them in case a user needs to.
The list of courses will be divided into two sections for user courses and public courses. Same will be applicable for assignments also.

== Design of Solution ==

Task 1: "Include others' items" checkbox is checked by default on Courses and Assignments manage contents page. At first it might seem that the design solution to improve latency is to uncheck this checkbox by default on page load. However, it's not the hard coded value of this checkbox that leads to latency of the page load. We need to further investigate what causes this latency.

On course tab, load assignment (children) nodes only when user expands the course listing.

Task 2: Assignments are preloaded along with the page for the course. Whenever you click on the course, there is an asynchronous AJAX call to the server to fetch the details of the assignment. To solve this problem, we would need to implement a function which would make an AJAX call to the server to fetch the assignment details in JSON format. For this, need to make changes in tree_display.jsx file to dynamically fetch the data.

===Use Case Diagrams===
[[File:Current_Implemenation.png]]

[[File:Proposed_Implementation.png]]

=== Database Design ===
This project is involved with improving the performance of the Course and Assignment Listing page. The work is majorly in reactJS and there is no necessity to change the current database design.

== Testing ==
=== Manual Testing ===
We will be performing manual testing for changes in the UI. Detailed steps are as follows:

1. Course listing is a view used by instructors & TAs to view courses. As the instructor & TAs login to expertiza, they would see that checkbox (“include others’ items”) to include public courses by other members, in the listing is unchecked by default. Similar to course listing, there is an assignment listing as well to be tested.

2. On course tab, load assignment (children) nodes only when user expands the course listing. They are not loaded when the page is loaded at the start.

3. The logged in user’s courses and others’ public courses are listed in two different sections. One section for each type. Section listing others’ public courses are displayed only when user selects the checkbox “include others’ items.

4. Similarly, the logged in user’s assignments and others’ public assignments are listed in two different sections. Section listing public assignments are be displayed only when user selects the checkbox “include others’ items”.

=== Automated Testing ===
For changes in the backend, we will be using Capybara to write automated tests. This would include tests for changes in the tree_display controller and student_task controller.

File:Proposed Implementation.png

2016-04-11T21:59:08Z

Nkerkar: Proposed_Implementation.png

Proposed_Implementation.png

File:Current Implemenation.png

2016-04-11T21:58:32Z

Nkerkar: Current Implementation

Current Implementation

CSC/ECE 517 Spring 2016 E1629 Performance improvement for Course & Assignment Listing

2016-04-05T02:20:12Z

Nkerkar: /* Design of Solution */

CSC/ECE 517 Spring 2016

2016-04-05T02:15:27Z

Nkerkar: /* Final Project Design Document */

==Writing Assignment 1==
*[[CSC/ECE 517 Spring 2016/Active Job]]

==Calibration Assignment Submissions==
*[[Calibration Assignment Submission (OmniAuth)]]
*[[Calibration Assignment Submission (Patch_verb)]]

==Writing Assignment 2==
*[[CSC/ECE 517 Spring 2016/Mozilla Implement HTML5 form validation]]

*[[CSC/ECE_517_Spring_2016/Mozilla Conforming Image Loading to HTML 5 Specs]]

*[[CSC/ECE 517 Spring 2016/Functional Tests for Questionnaire Controller]]

*[[CSC/ECE 517 Spring 2016 / Expertiza Self-Review Feature]]

*[[CSC/ECE 517 Spring 2016/Refactor different question types from quiz feature]]

*[[CSC/ECE 517 Spring 2016/Implement private browsing]]

*[[CSC/ECE 517 Spring 2016/Write automated tests for WebDriver]]

*[[CSC/ECE 517 Spring 2016/Refactor sign_up_sheet_controller.rb and sign_up_topic.rb]]

*[[CSC/ECE 517 Spring 2016/Implement Common Parts of the CSSOM API]]

*[[CSC/ECE 517 Spring 2016/Functional tests for assignment creation function]]

*[[CSC/ECE 517 Spring 2016/E1604. Functional tests for Calibration function]]

*[[CSC/ECE 517 Spring 2016/Refactor and write unit tests for question type.rb]]

*[[CSC/ECE 517 Spring 2016/Refactor response controller]]

*[[CSC/ECE_517_Spring_2016_OSS_M1606]]

*[[CSC/ECE_517_Spring_2016/OSS_E1601]]

==Final Project Design Document==
*[[CSC/ECE 517 Spring 2016 M1601 Implement HTML5 form validation]]
*[[CSC/ECE 517 Spring 2016 M1602 Make image loads conform with the spec]]
*[[CSC/ECE 517 Spring 2016 E1624 Compose functional tests for Suggest topics of student functionality ]]
*[[CSC/ECE 517 Spring 2016 E1631 Team-based reviewing]]
*[[CSC/ECE 517 Spring 2016 E1628 Visualization]]
*[[CSC/ECE 517 Spring 2016 E1625 Show confidence ratings for grade based on reputations of reviewers]]
*[[CSC/ECE 517 Spring 2016 E1629 Performance improvement for Course & Assignment Listing]]

CSC/ECE 517 Spring 2016

2016-04-05T02:14:54Z

Nkerkar: /* Final Project Design Document */

==Writing Assignment 1==
*[[CSC/ECE 517 Spring 2016/Active Job]]

==Calibration Assignment Submissions==
*[[Calibration Assignment Submission (OmniAuth)]]
*[[Calibration Assignment Submission (Patch_verb)]]

==Writing Assignment 2==
*[[CSC/ECE 517 Spring 2016/Mozilla Implement HTML5 form validation]]

*[[CSC/ECE_517_Spring_2016/Mozilla Conforming Image Loading to HTML 5 Specs]]

*[[CSC/ECE 517 Spring 2016/Functional Tests for Questionnaire Controller]]

*[[CSC/ECE 517 Spring 2016 / Expertiza Self-Review Feature]]

*[[CSC/ECE 517 Spring 2016/Refactor different question types from quiz feature]]

*[[CSC/ECE 517 Spring 2016/Implement private browsing]]

*[[CSC/ECE 517 Spring 2016/Write automated tests for WebDriver]]

*[[CSC/ECE 517 Spring 2016/Refactor sign_up_sheet_controller.rb and sign_up_topic.rb]]

*[[CSC/ECE 517 Spring 2016/Implement Common Parts of the CSSOM API]]

*[[CSC/ECE 517 Spring 2016/Functional tests for assignment creation function]]

*[[CSC/ECE 517 Spring 2016/E1604. Functional tests for Calibration function]]

*[[CSC/ECE 517 Spring 2016/Refactor and write unit tests for question type.rb]]

*[[CSC/ECE 517 Spring 2016/Refactor response controller]]

*[[CSC/ECE_517_Spring_2016_OSS_M1606]]

*[[CSC/ECE_517_Spring_2016/OSS_E1601]]

==Final Project Design Document==
*[[CSC/ECE 517 Spring 2016 M1601 Implement HTML5 form validation]]
*[[CSC/ECE 517 Spring 2016 M1602 Make image loads conform with the spec]]
*[[CSC/ECE 517 Spring 2016 E1624 Compose functional tests for Suggest topics of student functionality ]]
*[[CSC/ECE 517 Spring 2016 E1631 Team-based reviewing]]
*[[CSC/ECE 517 Spring 2016 E1628 Visualization]]
*[[CSC/ECE 517 Spring 2016 E1625 Show confidence ratings for grade based on reputations of reviewers]]
*[[CSC/ECE_517 Spring 2016 E1629 Performance improvement for Course & Assignment Listing]]

CSC/ECE 517 Spring 2016 E1629 Performance improvement for Course & Assignment Listing

2016-04-05T02:04:25Z

Nkerkar: /* Design of Solution */

CSC/ECE 517 Spring 2016 E1629 Performance improvement for Course & Assignment Listing

2016-04-05T00:59:51Z

Nkerkar:

CSC/ECE 517 Spring 2016/Refactor user course participant and assignment participant model

2016-03-24T01:19:15Z

Nkerkar:

== 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: Duplicated export_fields method===
This method exports the headers of the csv file to be downloaded. The method export_fields occurs in various models with different implementations. However, the implementation for this method had been duplicated in course_participant and user models. This led to the disadvantage of having redundant code in the system and an inefficient coding practice. User model represents the base class and other models inherit from this class. Hence, deleting the method from User model made little sense. The refactoring invoked the User models export_fields method from within the export_fields of course_participant. Since the export_file controller calls this method on the course_participant object, there wasn't a choice to delete the method completely. This refactoring has the advantage that code is reused and the proper object is tied to the header fields instead of just the generic User object.

===Refactoring: Breaking get_user_list method===
This method contained a lot of if-else statements that executed different codes depending on the type of user. The refactoring broke the large method into smaller methods, one for each user type. This improved the modularity of the code. It made the code more manageable and understandable from maintenance point of view (separation of responsibilities).

===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.
<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>

===Refactoring: topic_name in Participant.rb===
In participant.rb model, there is a method topic_name. However, this method doesn't seem to be required here. A course participant cannot have a topic. If it is an assignment participant object, we can find which team that participant is in first, then find which topic is this team holding. In fact, there is a topic_name field in SignedUpTeam class. On thorough investigation, we found that the method has no caller, which is the right implementation in any case. Hence, the refactoring involved deleting this method altogether. The refactoring was verified by the fact that none of the existing test cases failed.

==Testing==
Expertiza mainly uses RSpec for testing and has a comprehensive set of test cases overall. However, there are some models that do not have the required rspecs. We have included test cases for all the refactoring that we have done. We also ensured that the existing test cases do not fail.

===Test adding attr_accessible to class to improve security===
The earlier version of Expertiza did not have an rspec for the CourseParticipant model. We added an rspec named course_participant_spec.rb as the rspec for this model covering tests for the refactorings done in this project. The attr_accessible improves security by allowing only those fields that are in the list of attr_accessible macro for mass assignment. We test this by using “should allow_mass_assignment_of” and “should_not allow_mass_assignment_of”. Those attributes that are not in the list of attr_accessible will pass the test only when tested as “should_not allow_mass_assignment_of” and the attributes that are in the list of attr_accessible will pass the test only when tested as “should allow_mass_assignment_of”. The following is the code snippet of the test case.
<code>
describe "#accessible attributes" do
it { should allow_mass_assignment_of(:can_submit) }
it { should allow_mass_assignment_of(:can_review) }
it { should allow_mass_assignment_of(:user_id) }
it { should allow_mass_assignment_of(:parent_id) }
it { should allow_mass_assignment_of(:submitted_at) }
it { should allow_mass_assignment_of(:permission_granted) }
it { should allow_mass_assignment_of(:penalty_accumulated) }
it { should allow_mass_assignment_of(:grade) }
it { should allow_mass_assignment_of(:type) }
it { should allow_mass_assignment_of(:handle) }
it { should allow_mass_assignment_of(:time_stamp) }
it { should allow_mass_assignment_of(:digital_signature) }
it { should allow_mass_assignment_of(:duty) }
it { should allow_mass_assignment_of(:can_take_quiz) }
end
</code>

===Test the export_fields method===
This refactoring happened in the CourseParticipant model. The duplication is removed and the export_fields method is kept only in the User model and a call to the User model is made from CourseParticipant. To test this method, we test the functionality of this method as there is no test case for this method in the previous version. The export_fields method takes an argument of options which is a list of all the fields that the end user wants to export as course participant data and it returns the corresponding fields by making a call to User model. The ideal test case will tes only the export_fields in the course_participant model and not the one in user model. This is in line with the strategy of testing incoming query messages by making assertions about what they send back. Therefore, for a given set of options, only the required right set of fields are to be returned. We expect that the returned list of fields will match the ones the method asked using the options field. Below is a sample test snippet.
<code>
describe "#export_fields" do
it "returns export fields for a csv based on radio buttons checked" do
options = {"personal_details"=>"true", "role"=>"true", "parent"=>"true", "email_options"=>"true", "handle"=>"true"}
fields = ["name", "full name", "email", "role", "parent", "email on submission", "email on review", "email on metareview", "handle"]
expect(CourseParticipant.export_fields(options)).to match_array(fields)
end
</code>

===Test refactoring of get_user_list method into smaller methods===
In order to test get_user_list, we create a dummy object of User and set its role as instructor and fetch the user list of that instructor which returns the list of participants of that instructor’s course. The list could be empty or non empty depending on the user object.
<code>
describe "#get_user_list" do
it 'user list' do
@role = Role.new name: "Instructor"
user1 = User.new name: "instructor4", role: @role
expect(user1.get_user_list).to be_empty
expect(user1.get_users_instructor)
end
end
</code>