CSC/ECE 517 Spring 2016/E1720. UI issues/fixes
This wiki page is for the description of changes made according to the specification of E1720 OSS assignment for Spring 2016.
Peer Review Information
For testing the changes made, the following credentials are recommended:
- Instructor Login: username: instructor6 password: password
The above user is suggested for testing because is is unlike to run into issues that are unrelated to our work.
This project does compile. Reviewers will need to perform "rake db:migrate" as we've added a database table to support notifications.
The testing plan is at the end of this document. There were also specs added; however, they are in the Git repository. Design principles were not needed since we mostly modified existing work. However, issue #702 is using the Factory Method design pattern, and our work added content to the existing pattern.
Introduction
Background
Expertiza is a web portal which can be used to manage assignments related to a course. It provides a platform to view assignments, manage teams, select topics and work improvement through anonymous peer reviews. For the instructor it provides complete control to create assignments, view reviews submitted and provide feedback. The instructors also have an option to publish the students work based on the rights provided by the student.
Problem Statement
The following were the tasks identified to be accomplish through this project.
- Issue #702: Add another institution_id to course table and The Create Course page needs to be fixed to tell the creator to specify the institution (course/_course.html.erb).
- Issue #316: Remove "Actions" column on signup sheet in a completed assignment (sign_up_sheet/list.html.erb).
- Issue #295: Add a confirmation on deleting an assignment on the admin screen (tree_display/actions/_shared_actions.html.rb).
- Issue #256: Add a one-time notification at the top of the page which links to an article on the details of the change (it might make sense to put a javascript module in the site_controllers/index.html.rb and let other pages call this).
Files Modified in this Project
The following files were modified for this project.
- app/assets/javascripts/tree_display.jsx
- app/controllers/auth_controller.rb
- app/controllers/course_controller.rb
- app/controllers/tree_display_controller.rb
- app/models/assignment_node.rb
- app/models/course.rb
- app/models/course_node.rb
- app/models/institution.rb
- app/views/course/_course.html.erb
- app/views/layouts/application.html.erb
- app/views/notifications/index.html.erb
- app/views/shared/_flash_notifications.html.erb
- app/view/sign_up_sheet/_table_line.html.erb
- app/views/tree_display/list.html.erb
- app/views/tree_display/confirm.html.erb
- app/views/tree_display/new.html.erb
- app/views/tree_display/edit.html.erb
- database table "notification"
Solutions implemented and Delivered
Issue #702:
This task was pretty involved and called for integrating an institution_id into the course table and updating the Course pages to specify the institution. We modified several parts of the program in order to accomplish this task.
On the controller app/controllers/course_controller we edited the update definition and the create definition to store the institution_id into the course table.
def update
@course = Course.find(params[:id])
if params[:course][:directory_path] and @course.directory_path != params[:course][:directory_path]
begin
FileHelper.delete_directory(@course)
rescue
flash[:error] = $ERROR_INFO
end
begin
FileHelper.create_directory_from_path(params[:course][:directory_path])
rescue
flash[:error] = $ERROR_INFO
end
end
@course.name = params[:course][:name]
@course.institutions_id = params[:course][:institutions_id]
@course.directory_path = params[:course][:directory_path]
@course.info = params[:course][:info]
@course.save
undo_link("The course \"#{@course.name}\" has been updated successfully.")
redirect_to controller: 'tree_display', action: 'list'
end
def create
@course = Course.new(name: params[:course][:name], institutions_id: params[:course][:institutions_id], directory_path: params[:course][:directory_path], info: params[:course][:info], private: params[:course][:private])
end
On the model app/models/course.rb we created a relationship where course belongs to institution. This is to allow the linkage of the two tables and allow the drop down to work in the view.
belongs_to :institution, class_name: 'Institution', foreign_key: 'institutions_id'
On the model app/models/institution.rb we also created the relationship where institution has many courses. This is also to allow linkage of the two tables for the drop down to work in the view.
class Institution < ActiveRecord::Base attr_accessible :name has_many :courses validates_length_of :name, minimum: 1 validates_uniqueness_of :name end
On the view app/views/course/_course.html.erb we added the collection box to display the linked name from the institution table to the institution_id in the course table. This completed the ability to select and store the institution_id into the database.
<p><label for="institution_name">Institution Name</label><br/>
<%= select("course", "institutions_id", Institution.all.collect{ |c| [ c.name, c.id] }) %></p>
In order to properly display the institution_id associated with the courses on the view app/views/tree_display/list.html.erb it was necessary to understand the Factory Method design pattern. A subclass of app/models/node.rb is app/models/course_node.rb and app/models/assigment_node.rb. Both need similar functions to be used by app/assets/javascripts/tree_display.jsx for displaying rows of courses and assignments. This was added to both, but only used by app/models/course_node.rb:
def get_institution_id # Course.find(self.node_object_id).course_id @course = Course.find(self.node_object_id) unless @course @course.institutions_id end
app/controllers/tree_display_controller.rb was modified to include a local variable for used by the view in displaying the institution_id.
def update_tmp_obj(tmp_object, node)
tmp={
"directory" => node.get_directory,
"creation_date" => node.get_creation_date,
"updated_date" => node.get_modified_date,
"institution" => Institution.where(:id => node.get_institution_id),
"private" => node.get_instructor_id == session[:user].id ? true : false
}
tmp_object.merge!(tmp)
end
app/assets/javascript/tree_display.jsx was also modified to show different rows between courses and assignments, since only courses were required to show the institution_id. This was added in order to show the institution_id which is displayed on app/views/tree_display/list.html.erb:
jQuery.each(this.props.data, function (i, entry) {
if (((entry.name && entry.name.indexOf(_this.props.filterText) !== -1) ||
(entry.directory && entry.directory.indexOf(_this.props.filterText) !== -1) ||
(entry.creation_date && entry.creation_date.indexOf(_this.props.filterText) !== -1) ||
(entry.instructor && entry.instructor.indexOf(_this.props.filterText) !== -1) ||
(entry.institution && entry.institution.indexOf(_this.props.filterText) !== -1) ||
(entry.updated_date && entry.updated_date.indexOf(_this.props.filterText) !== -1)) &&
(entry.private == true || entry.type == 'FolderNode')) {
_rows.push(<ContentTableRow
key={entry.type+'_'+(parseInt(entry.nodeinfo.id)*2).toString()+'_'+i}
id={entry.type+'_'+(parseInt(entry.nodeinfo.node_object_id)*2).toString()+'_'+i}
name={entry.name}
directory={entry.directory}
instructor={entry.instructor}
institution={entry.institution}
creation_date={entry.creation_date}
updated_date={entry.updated_date}
actions={entry.actions}
is_available={entry.is_available}
course_id={entry.course_id}
max_team_size={entry.max_team_size}
is_intelligent={entry.is_intelligent}
require_quiz={entry.require_quiz}
dataType={_this.props.dataType}
//this is just a hack. All current users courses are marked as private during fetch for display purpose.
private={entry.private}
allow_suggestions={entry.allow_suggestions}
has_topic={entry.has_topic}
rowClicked={_this.handleExpandClick}
newParams={entry.newParams}
/>)
Issue #316:
This task called for removing the 'Actions' column on the signup sheet for assignments that have been completed (sign_up_sheet/list.html.erb).
(sign_up_sheet/list.html.erb) renders app/view/sign_up_sheet/_table_line.html.erb and an if statement is used to not show finished assignements.
<% if @assignment.current_stage_name(@topic_id) != 'Finished' %>
<%= render :partial => '/sign_up_sheet/actions', :locals => {:i=>i,:topic=>topic, :is_suggested_topic=>is_suggested_topic ||= false} %>
<% end %>
Issue #295:
This task called for adding a confirmation when deleting an assignment on the admin screen. While the instructions indicated (app/views/tree_display/actions/_shared_actions.html.rb), this file is no longer in use in the project. In fact, all assignments and courses on (app/views/tree_display/list.html.erb) are displayed by (app/assets/javascripts/tree_display.jsx) and handled in JavaScript. Moreover, the normal confirmation actions are overridden by (app/assets/javascript/userDeleteConfirmBox.js). The overridden confirmation actions were not compatible with the JavaScript deletion link. We overcame this issue by creating a new view (app/views/tree_display/confirm.html.erb), and redirecting the deletion link to the view for confirmation. We passed the nodeType (assignment or course) and id of the node to be deleted in the URL to be stored by the controller as local variables for use on the confirmation view. The selected assignment (or course) is deleted only after confirmed as per the requirement.
The part of the code that redirected deletions for assignments or courses to the confirmation page is in 'app/assets/javascripts/tree_display.jsx'. (Item in bold was modified)
if (this.props.is_available || newNodeType == 'questionnaires') {
moreContent.push(
<a title="Edit" href={"/"+newNodeType+"/"+(parseInt(this.props.id)/2).toString()+"/edit"}><img src="/assets/tree_view/edit-icon-24.png" /></a>
<a title="Delete" href={"/tree_display/confirm?id="+(parseInt(this.props.id)/2).toString()+"&nodeType="+newNodeType}><img src="/assets/tree_view/delete-icon-24.png" /></a>
<a title={this.props.private? "Make public" : "Make private"} href={"/"+newNodeType+"/toggle_access?id="+(parseInt(this.props.id)/2).toString()}><img src={"/assets/tree_view/lock-"+(this.props.private? "off-" : "")+"disabled-icon-24.png"} /></a>
)
}
Original code (Delete link without confirmation.)
<a title="Delete" href={"/"+newNodeType+"/delete?id="+(parseInt(this.props.id)/2).toString()}><img src="/assets/tree_view/delete-icon-24.png" /></a>
Definition added to 'app/controllers/tree_display_controller.rb' for business logic.
def confirm @id = params[:id] @nodeType = params[:nodeType] end
Here we see that the code inside of the new view 'app/views/tree_display/confirm.html.erb'.
<% if @nodeType == 'course' %>
<h1>Are you sure you want to delete this course?</h1>
<% else %>
<h1>Are you sure you want to delete this assignment?</h1>
<% end %>
<table align="center">
<tr align="center"><tr></tr><tr></tr>
<td align="center" width="120px" style="font-size: xx-large"><%= link_to 'NO', list_tree_display_index_url %></td>
<td align="center" width="120px" style="font-size: xx-large">
<% if @nodeType == 'course' %>
<%= link_to 'YES', :controller => :course, :action => :delete, :id => @id %>
<% else %>
<%= link_to 'YES', :controller => :assignments, :action => :delete, :id => @id %>
<% end %>
</td>
</tr>
</table>
Issue #256:
This task called for adding a one-time notification at the top of the page to show changes/notifications. It necessitated adding a table to the database called notifications. Therefore, when conducting peer reviews, a migration must be performed to add the new table. Then we used scaffolding to add notifications (controller, model, and views). Next we added a link on the view (app/view/tree_display/index.html.erb) to go to the view (app/view/notification/index.html.erb) in order to manage notifications.
A migration to create a notifications table was added.
class CreateNotifications < ActiveRecord::Migration
def change
create_table :notifications do |t|
t.string :subject
t.text :description
t.date :expiration_date
t.boolean :active_flag
t.timestamps null: false
end
end
end
Using scaffolding, a new index was added app/view/notifications/index.html.erb.
<h1>Listing Notifications</h1>
<table>
<thead>
<tr>
<th>Subject</th>
<th>Description</th>
<th>Expiration date</th>
<th>Active flag</th>
<th colspan="3"></th>
</tr>
</thead>
<tbody>
<% @notifications.each do |notification| %>
<tr>
<td><%= notification.subject %></td>
<td><%= notification.description %></td>
<td><%= notification.expiration_date %></td>
<td><%= notification.active_flag %></td>
<td><%= link_to 'Show', notification %></td>
<td><%= link_to 'Edit', edit_notification_path(notification) %></td>
<td><%= link_to 'Destroy', notification, method: :delete, :confirm => 'Are you sure?' %></td>
</tr>
<% end %>
</tbody>
</table>
<br>
<%= link_to 'New Notification', new_notification_path %>
Added link on page for managers to manage notifications app/view/tree_display/list.html.erb.
Notifications are added and modified in similar views app/view/tree_display/new.html.erb or app/view/tree_display/edit.html.erb.
When any type of user successfully logs in, flash notifications called from the controller app/controllers/auth_controller.rb.
def after_login(user)
session[:user] = user
AuthController.set_current_role(user.role_id, session)
flash[:notification] = 'This will display notifications on login'
redirect_to controller: AuthHelper.get_home_controller(session[:user]),
action: AuthHelper.get_home_action(session[:user])
end
Flash notifications are displayed once at login due to a line in the view app/view/layouts/application.html.erb which is used by all other views.
<%= render 'shared/flash_notifications' %>
Each individual flash notification that is active and is not expired is displayed through the use of the view app/view/shared/_flash_notifications.html.erb.
<% if (flash_message :notification) != nil %>
<% @notifications = Notification.where(:active_flag => true) %>
<% if !@notifications.nil? %>
<% @notifications.each do |notification| %>
<% if notification.expiration_date >= Date.today %>
<table style="background-color: #faebcc">
<tbody style="background-color: #faebcc"><br>
<tr><p style="color:black; font-width:bold; background-color: #faebcc"><%= notification.subject.upcase %></p></tr>
<tr><%= notification.description %></tr>
</tbody>
</table>
<% end %>
<% end %>
<% end %>
<% end %>
The notifications are displayed on whatever page the login screen goes to first depending of the type of user logging into the system. They are only display once and are gone whenever the page is refreshed or changed.
Testing Plan: Testing from the UI
UI Testing
The majority of the changes can be tested via the UI. Follow the instructions below to check the tasks:
- Issue #702: Add another institution_id to course table and The Create Course page needs to be fixed to tell the creator to specify the institution (course/_course.html.erb).
- Login as an instructor. It will automatically go to the manage courses page.
- Click the "New Public Course" or "New Private Course" button.
- You will notice a drop down for institutions is available.
- Create a new course, and the UI will automatically return to the list which displays the courses. You will see the institution listed.
- Click on the edit button and you will be able to modify the institution for the course.
- Issue #316: Remove "Actions" column on signup sheet in a completed assignment (sign_up_sheet/list.html.erb).
- Log in as any user or instructor.
- Go to the assignments list. Note: this is not the manage assignments for instructors.
- Click into a finished assignment. You will notice the actions column is gone.
- Click into a current assignment. You will notice the actions column appears.
- Issue #295: Add a confirmation on deleting an assignment on the admin screen (tree_display/actions/_shared_actions.html.rb).
- Login as an instructor. It will automatically go to the manage courses page.
- Click the Assignments link to switch the view from courses to assignments.
- Add a new assignment.
- Click the delete button for an assignment. Note: please add an assignment first. Most of the current assignments have dependencies that prevent them from being deleted... this is not an error.
- The UI will be directed to a page to confirm the deletion. Select "Yes" if you wish to delete the assignment.
- Issue #256: Add a one-time notification at the top of the page which links to an article on the details of the change (it might make sense to put a javascript module in the site_controllers/index.html.rb and let other pages call this).
- Login as an instructor. It will automatically go to the manage courses page.
- Click on the "Manage Notifications" link above the "Courses" and "Assignments" links. Note: The link was placed here because the system has numerous issues when adding menu items. It would require us to do more work that the entirety of this project to correct the menu additions in the superuser menu.
- You will be directed to a creation page for notifications. Note: in order for a notification to display the expiration date must be the current date or later, and the active checkbox must be selected.
- Log out once the notification is created.
- Log in using any account. The notification will display on the first page the user is shown. It will disappear when they change or reload the page. Note: instructors on the management page will retain the notification if they switch between Assignments, Courses, and Questionnaires. This is because the JavaScript does not actually reload the page.