CSC/CSC 517 Fall 2019/oss E1970
E1970. OSS project Oogie Boogie: User Experience & Confidence
This page provides a description of the Expertiza based OSS project.
About Expertiza
Expertiza is an open source project based on Ruby on Rails framework. Expertiza allows the instructor to create new assignments and customize new or existing assignments. It also allows the instructor to create a list of topics the students can sign up for. Students can form teams in Expertiza to work on various projects and assignments. Students can also peer review other students' submissions. Expertiza supports submission across various document types, including the URLs and wiki pages.
Problem Statement
The following tasks were accomplished in this project:
- Fix Issue #1467: Missing Header
- The ‘view submissions’ page of an assignment does not have an appropriate header.
- The header should be added to reflect text such as “Submissions for <assignment name>
- Fix Issue #1212: HTML text tag displayed in status-bar of text input box
- The New review page contains a spurious "p" before each criterion is shown
- The status p (paragraph) should be removed
- Fix Issue #1312: Assignments with “Use dropdown instead” cannot be saved.
- When an assignment is being created with a rubric that uses the “Use dropdown instead” option, it falsely indicates the save was successful.
- The save is not persisted when we review the assignment rubric. The option “Use dropdown” is not selected.
- The option should be saved in database and reflected on the review page.
Issue #1467: Missing Header
The ‘view submissions’ page of an assignment does not have an appropriate header. The header should be added to reflect text such as “Submissions for <assignment name>
Before
Functionality
- The view submission page currently has no title representing which homework submission the viewer is viewing, a title needs to be added into where the red circle is.
Solutions
- View
- The view shows what's presented to user of this page, if we would like to have additional information displayed on the page, we would need to find the corresponding location to add it first:
<% headers = {} %>
<% headers["Topic_name"] = "16%" if @assignment.topics? %>
<% if @assignment.max_team_size > 1 %>
<% headers["Team name"] = "14%" %>
<% headers["Team member(s)"] = "18%" %>
<% else %>
<% headers["Participant name"] = "18%" %>
<% end %>
<% headers["Submitted item(s)"] = nil %>
<!--E1877: table id changed -->
<table id ="submissionsTable" class="table table-striped" style="margin-top: 50px">
<thead>
<!--E1877: class="sorter-true" added to sort all attributes-->
<tr>
<% if @assignment.topics? %>
<th class="sorter-true" style="font-weight: bold; font-size: 15px;">Topic name</th>
<% end %>
<% if @assignment.max_team_size > 1 %>
<th class="sorter-true" style="font-weight: bold; font-size: 15px;">Team name</th>
<th class="sorter-true" style="font-weight: bold; font-size: 15px;">Team member(s)</th>
<% else %>
<th class="sorter-true" style="font-weight: bold; font-size: 15px;">Participant name</th>
<% end %>
<th class="sorter-true" style="font-weight: bold; font-size: 15px;">Links</th>
<th class="sorter-false" style="font-weight: bold; font-size: 15px;"></th>
</tr>
</thead>
In this case, the blank space above team E1877's change should be where the red circle stands.
After
- View change
- Since this page already uses `@assignment` in a lot of places, and we found it in the corresponding controller, so directly using it wouldn't pose additional problems as the query of the assignment is already done in the back-end.
<% headers = {} %>
<% headers["Topic_name"] = "16%" if @assignment.topics? %>
<% if @assignment.max_team_size > 1 %>
<% headers["Team name"] = "14%" %>
<% headers["Team member(s)"] = "18%" %>
<% else %>
<% headers["Participant name"] = "18%" %>
<% end %>
<% headers["Submitted item(s)"] = nil %>
<!--E1970: Added missing header (assignment name) -->
<h1><%= @assignment.name %></h1>
<!--E1877: table id changed -->
<table id ="submissionsTable" class="table table-striped" style="margin-top: 50px">
<thead>
<!--E1877: class="sorter-true" added to sort all attributes-->
<tr>
<% if @assignment.topics? %>
<th class="sorter-true" style="font-weight: bold; font-size: 15px;">Topic name</th>
<% end %>
<% if @assignment.max_team_size > 1 %>
<th class="sorter-true" style="font-weight: bold; font-size: 15px;">Team name</th>
<th class="sorter-true" style="font-weight: bold; font-size: 15px;">Team member(s)</th>
<% else %>
<th class="sorter-true" style="font-weight: bold; font-size: 15px;">Participant name</th>
<% end %>
<th class="sorter-true" style="font-weight: bold; font-size: 15px;">Links</th>
<th class="sorter-false" style="font-weight: bold; font-size: 15px;"></th>
</tr>
</thead>
Issue #1467: Missing Header
The ‘view submissions’ page of an assignment does not have an appropriate header. The header should be added to reflect text such as “Submissions for <assignment name>
Before
Functionality
- The view submission page currently has no title representing which homework submission the viewer is viewing, a title needs to be added into where the red circle is.
Solutions
- View
- The view shows what's presented to user of this page, if we would like to have additional information displayed on the page, we would need to find the corresponding location to add it first:
<% headers = {} %>
<% headers["Topic_name"] = "16%" if @assignment.topics? %>
<% if @assignment.max_team_size > 1 %>
<% headers["Team name"] = "14%" %>
<% headers["Team member(s)"] = "18%" %>
<% else %>
<% headers["Participant name"] = "18%" %>
<% end %>
<% headers["Submitted item(s)"] = nil %>
<!--E1877: table id changed -->
<table id ="submissionsTable" class="table table-striped" style="margin-top: 50px">
<thead>
<!--E1877: class="sorter-true" added to sort all attributes-->
<tr>
<% if @assignment.topics? %>
<th class="sorter-true" style="font-weight: bold; font-size: 15px;">Topic name</th>
<% end %>
<% if @assignment.max_team_size > 1 %>
<th class="sorter-true" style="font-weight: bold; font-size: 15px;">Team name</th>
<th class="sorter-true" style="font-weight: bold; font-size: 15px;">Team member(s)</th>
<% else %>
<th class="sorter-true" style="font-weight: bold; font-size: 15px;">Participant name</th>
<% end %>
<th class="sorter-true" style="font-weight: bold; font-size: 15px;">Links</th>
<th class="sorter-false" style="font-weight: bold; font-size: 15px;"></th>
</tr>
</thead>
In this case, the blank space above team E1877's change should be where the red circle stands.
After
- View change
- Since this page already uses `@assignment` in a lot of places, and we found it in the corresponding controller, so directly using it wouldn't pose additional problems as the query of the assignment is already done in the back-end.
<% headers = {} %>
<% headers["Topic_name"] = "16%" if @assignment.topics? %>
<% if @assignment.max_team_size > 1 %>
<% headers["Team name"] = "14%" %>
<% headers["Team member(s)"] = "18%" %>
<% else %>
<% headers["Participant name"] = "18%" %>
<% end %>
<% headers["Submitted item(s)"] = nil %>
<!--E1970: Added missing header (assignment name) -->
<h1><%= @assignment.name %></h1>
<!--E1877: table id changed -->
<table id ="submissionsTable" class="table table-striped" style="margin-top: 50px">
<thead>
<!--E1877: class="sorter-true" added to sort all attributes-->
<tr>
<% if @assignment.topics? %>
<th class="sorter-true" style="font-weight: bold; font-size: 15px;">Topic name</th>
<% end %>
<% if @assignment.max_team_size > 1 %>
<th class="sorter-true" style="font-weight: bold; font-size: 15px;">Team name</th>
<th class="sorter-true" style="font-weight: bold; font-size: 15px;">Team member(s)</th>
<% else %>
<th class="sorter-true" style="font-weight: bold; font-size: 15px;">Participant name</th>
<% end %>
<th class="sorter-true" style="font-weight: bold; font-size: 15px;">Links</th>
<th class="sorter-false" style="font-weight: bold; font-size: 15px;"></th>
</tr>
</thead>
Code improvements
- Introduced a constant VERSIONS_PER_PAGE and assigned the value 25 to it. The pagination algorithm for VersionsController displays at most 25 versions in a page. The existing implementation uses the value 25 straight in the code and there are few problems associated with such an approach.
- It is not easy to understand what 25 is unless the programmer takes a close look at the code.
- In case if the value 25 is used at more than one places and in future a new requirement comes to show at most 30 versions in a page, all the values will have to be modified. It is not very DRY.
- The VersionsController was overriding AccessHelper - action_allowed? method to return true in all the cases. This was violating the whole purpose of the method action_allowed?. The purpose of this method is to determine whether the user who is triggering a CRUD operation is allowed to do so. So when the current user invokes a CRUD operation, the action_allowed? method is invoked first and if the method returns true the CRUD operation is triggered or else the user is intimated with a message and gracefully exited. Hence, when the action_allowed? method is overridden to return true always, it results in providing unauthorized access to certain users.
def action_allowed?
true
end
- With the new implementation the AccessHelper - action_allowed? method has been modified in such a way that unauthorized access is prevented. As per the new algorithm, 'new', 'create', 'edit', 'update' cannot be invoked by any user. These operations can be accessed only by ‘papertrail’ gem. Only an ‘Administrator’ or ‘Super-Administrator’ can call 'destroy_all' method. All the other methods are accessible to ‘Administrator’, ‘Super-Administrator’, ‘Instructor’, ‘Teaching Assistant’ and ‘Student’.
def action_allowed?
case params[:action]
when 'new', 'create', 'edit', 'update'
#Modifications can only be done by papertrail
return false
when 'destroy_all'
['Super-Administrator',
'Administrator'].include? current_role_name
else
#Allow all others
['Super-Administrator',
'Administrator',
'Instructor',
'Teaching Assistant',
'Student'].include? current_role_name
end
end
Automated Testing using RSPEC
The current version of expertiza did not have any test for VersionsController. Using the test driven development(TDD) approach, we have added an exhaustive set of RSPEC tests for VersionsController, to test all the modifications we have done to the code of the controller class. The tests use double and stub features of rspec-rails gem, to fake the log in by different users - Administrator, Instructor, Student etc. The tests can be executed "rpec spec" command as shown below.
user-expertiza $rspec spec . . . Finished in 5.39 seconds (files took 25.33 seconds to load) 66 examples, 0 failures Randomized with seed 19254 . .
Testing from UI
Following are a few testcases with respectto our code changes that can be tried from UI: 1. To go to versions index page, type in the following url after logging in:
http://152.46.16.81:3000/versions
2. After logging in as student/instructor or admin : Try accessing the new, create, edit, update actions. These actions are not allowed to any of the users.
http://152.46.16.81:3000/versions/new This calls the new action. In the current production version of expertiza, it is unhandled and application gives a default 404 page.
3. Another feature that can be tested from UI is Pagination. Try searching for a user's versions and see if the results are paginated or not. Search here:
http://152.46.16.81:3000/versions/search
4. Visit the same URL as step 3, you should see only the students under that instructor in the users dropdown.
References
- Expertiza on GitHub
- GitHub Project Repository Fork
- The live Expertiza website
- Demo link
- Expertiza project documentation wiki
- Rspec Documentation
- Clean Code: A handbook of agile software craftsmanship. Author: Robert C Martin