Better "Courses" menu, updated sort, count, etc.
* Doesn't display enrollments with identical created_at and updated_at fields (SIS garbage) * Always displays "See all Courses" link with a count of all current enrollments. * No more duplicate links to /courses, i.e. "... and 12 more" "See all enrollments" * Displays 12 instead of 8 * Sorts alphabetically after sorting by rank_sortable and state_sortable * Moved the logic out of the view into the user model and some helpers Change-Id: Ibcd274cae8e192585ad77f1bad6e0b54da041c8b Reviewed-on: https://gerrit.instructure.com/5822 Tested-by: Hudson <hudson@instructure.com> Reviewed-by: Jon Jensen <jon@instructure.com>
This commit is contained in:
Ryan Florence
parent
16f6757044
commit
e85e98cb25
|
|
@ -509,4 +509,55 @@ var I18n = I18n || {};
|
|||
end
|
||||
super
|
||||
end
|
||||
|
||||
def menu_enrollments_locals
|
||||
max = 12
|
||||
{
|
||||
:collection => @current_user.menu_data[:enrollments][0, max].sort_by{|e| e[:enrollment].long_name },
|
||||
:collection_size => @current_user.menu_data[:enrollments_count],
|
||||
:partial => "shared/menu_enrollment",
|
||||
:max_to_show => max,
|
||||
:more_link_for_over_max => courses_path,
|
||||
:title => t('#menu.my_courses', "My Courses"),
|
||||
:course_name_counts => @current_user.menu_data[:course_name_counts],
|
||||
:link_text => raw(t('#layouts.menu.view_all_enrollments', 'View all courses'))
|
||||
}
|
||||
end
|
||||
|
||||
def menu_groups_locals
|
||||
{
|
||||
:collection => @current_user.cached_current_group_memberships,
|
||||
:partial => "shared/menu_group_membership",
|
||||
:collection_size => @current_user.cached_current_group_memberships.length,
|
||||
:max_to_show => 8,
|
||||
:more_link_for_over_max => groups_path,
|
||||
:title => t('#menu.current_groups', "Current Groups"),
|
||||
:course_name_counts => @current_user.menu_data[:course_name_counts],
|
||||
:link_text => raw(t('#layouts.menu.view_all_groups', 'View all groups'))
|
||||
}
|
||||
end
|
||||
|
||||
def menu_accounts_locals
|
||||
{
|
||||
:collection => @current_user.accounts,
|
||||
:partial => "shared/menu_account",
|
||||
:collection_size => @current_user.accounts.length,
|
||||
:max_to_show => 8,
|
||||
:more_link_for_over_max => accounts_path,
|
||||
:title => t('#menu.managed_accounts', "Managed Accounts"),
|
||||
:course_name_counts => @current_user.menu_data[:course_name_counts],
|
||||
:link_text => raw(t('#layouts.menu.view_all_accounts', 'View all accounts'))
|
||||
}
|
||||
end
|
||||
|
||||
def show_home_menu?
|
||||
@current_user.set_menu_data(session[:enrollment_uuid])
|
||||
[
|
||||
@current_user.menu_data[:enrollments],
|
||||
@current_user.accounts,
|
||||
@current_user.cached_current_group_memberships,
|
||||
@current_user.enrollments.ended
|
||||
].any?{ |e| e.respond_to?(:count) && e.count > 0 }
|
||||
end
|
||||
|
||||
end
|
||||
|
|
|
|||
|
|
@ -20,7 +20,7 @@ class User < ActiveRecord::Base
|
|||
include Context
|
||||
|
||||
attr_accessible :name, :short_name, :time_zone, :show_user_services, :gender, :visible_inbox_types, :avatar_image, :subscribe_to_emails, :locale
|
||||
attr_accessor :original_id
|
||||
attr_accessor :original_id, :menu_data
|
||||
|
||||
before_save :infer_defaults
|
||||
serialize :preferences
|
||||
|
|
@ -1849,4 +1849,58 @@ class User < ActiveRecord::Base
|
|||
# there's better solutions in AR 3.
|
||||
# See also e.g., http://makandra.com/notes/983-dynamic-conditions-for-belongs_to-has_many-and-has_one-associations
|
||||
has_many :submissions_for_given_assignments, :include => [:assignment, :submission_comments], :conditions => 'submissions.assignment_id IN (#{Api.assignment_ids_for_students_api.join(",")})', :class_name => 'Submission'
|
||||
|
||||
def set_menu_data(enrollment_uuid)
|
||||
max_to_show = 8
|
||||
coalesced_enrollments = []
|
||||
course_name_counts = {}
|
||||
has_completed_enrollment = false
|
||||
cached_enrollments = self.cached_current_enrollments(:include_enrollment_uuid => enrollment_uuid)
|
||||
cached_enrollments.each do |e|
|
||||
|
||||
# SIS "garbage" data, user auto-enrolled but has never done anything with it
|
||||
next if e.updated_at == e.created_at
|
||||
|
||||
next if e.state_based_on_date == :inactive
|
||||
|
||||
if e.state_based_on_date == :completed
|
||||
has_completed_enrollment = true
|
||||
next
|
||||
end
|
||||
|
||||
if !e.course
|
||||
coalesced_enrollments << {
|
||||
:enrollment => e,
|
||||
:sortable => [e.rank_sortable, e.state_sortable, e.long_name],
|
||||
:types => [ e.readable_type ]
|
||||
}
|
||||
end
|
||||
|
||||
existing_enrollment_info = coalesced_enrollments.find { |en|
|
||||
# coalesce together enrollments for the same course and the same state
|
||||
!e.course.nil? && en[:enrollment].course == e.course && en[:enrollment].workflow_state == e.workflow_state
|
||||
}
|
||||
|
||||
if existing_enrollment_info
|
||||
existing_enrollment_info[:types] << e.readable_type
|
||||
existing_enrollment_info[:sortable] = [existing_enrollment_info[:sortable] || [999,999, 999], [e.rank_sortable, e.state_sortable, 0 - e.id]].min
|
||||
else
|
||||
coalesced_enrollments << { :enrollment => e, :sortable => [e.rank_sortable, e.state_sortable, 0 - e.id], :types => [ e.readable_type ] }
|
||||
course_name_counts[e.course.name] ||= 0
|
||||
course_name_counts[e.course.name] += 1
|
||||
end
|
||||
end
|
||||
coalesced_enrollments = coalesced_enrollments.sort_by{|e| e[:sortable] || [999,999, 999] }
|
||||
@menu_data = {
|
||||
:enrollments => coalesced_enrollments,
|
||||
:enrollments_count => cached_enrollments.length,
|
||||
:course_name_counts => course_name_counts,
|
||||
:has_completed_enrollment => has_completed_enrollment,
|
||||
}
|
||||
end
|
||||
|
||||
def has_ended_enrollments?
|
||||
self.enrollments.ended.length || self.menu_data[:has_completed_enrollment]
|
||||
end
|
||||
|
||||
end
|
||||
|
|
|
|||
|
|
@ -80,67 +80,10 @@
|
|||
<ul role="navigation" id="menu">
|
||||
<%- cache([@current_user, 'home-menu-15m'], :expires_in => 15.minutes) do -%>
|
||||
<li class="menu-item">
|
||||
<%# if any of these have anything in them, then we should show a dropdown on the 'home' menu item %>
|
||||
<%- if [ @current_user.cached_current_enrollments(:include_enrollment_uuid => session[:enrollment_uuid]),
|
||||
@current_user.accounts,
|
||||
@current_user.cached_current_group_memberships,
|
||||
@current_user.enrollments.ended ].any?{ |e| e.respond_to?(:count) && e.count > 0 } -%>
|
||||
<a href="<%= dashboard_path %>" class="menu-item-title">
|
||||
<%= @current_user.cached_current_group_memberships.empty? ? t('links.courses', "Courses") : t('links.courses_and_groups', "Courses & Groups") -%>
|
||||
<span class="menu-item-title-icon"></span></a>
|
||||
<div class="menu-item-drop">
|
||||
<table cellspacing="0">
|
||||
<tr>
|
||||
<%
|
||||
max_to_show = 8 # Need to keep this low for users with limited screen real estate
|
||||
coalesced_enrollments = []
|
||||
course_name_counts = {}
|
||||
found_completed = false
|
||||
@current_user.cached_current_enrollments(:include_enrollment_uuid => session[:enrollment_uuid]).each do |e|
|
||||
next if e.state_based_on_date == :inactive
|
||||
if e.state_based_on_date == :completed
|
||||
found_completed = true
|
||||
next
|
||||
end
|
||||
coalesced_enrollments << { :enrollment => e, :sortable => [e.rank_sortable, e.state_sortable, 0 - e.id], :types => [ e.readable_type ] } if !e.course
|
||||
existing_enrollment_info = coalesced_enrollments.find { |en|
|
||||
# coalesce together enrollments for the same course and the same state
|
||||
!e.course.nil? && en[:enrollment].course == e.course && en[:enrollment].workflow_state == e.workflow_state
|
||||
}
|
||||
if existing_enrollment_info
|
||||
existing_enrollment_info[:types] << e.readable_type
|
||||
existing_enrollment_info[:sortable] = [existing_enrollment_info[:sortable] || [999,999, 999], [e.rank_sortable, e.state_sortable, 0 - e.id]].min
|
||||
else
|
||||
coalesced_enrollments << { :enrollment => e, :sortable => [e.rank_sortable, e.state_sortable, 0 - e.id], :types => [ e.readable_type ] }
|
||||
course_name_counts[e.course.name] ||= 0
|
||||
course_name_counts[e.course.name] += 1
|
||||
end
|
||||
end
|
||||
coalesced_enrollments = coalesced_enrollments.sort_by{|e| e[:sortable] || [999,999, 999] }
|
||||
[
|
||||
[coalesced_enrollments, "shared/menu_enrollment", t('menu.my_courses', "My Courses"), courses_path],
|
||||
[@current_user.cached_current_group_memberships, "shared/menu_group_membership", t('menu.current_groups', "Current Groups"), groups_path],
|
||||
[@current_user.accounts, "shared/menu_account", t('menu.managed_accounts', "Managed Accounts"), accounts_path]
|
||||
].each do |triple| %>
|
||||
<%= render(:partial => "shared/menu_section", :locals => {
|
||||
:collection => triple.first[0,max_to_show],
|
||||
:partial => triple[1],
|
||||
:collection_size => triple.first.length,
|
||||
:max_to_show => max_to_show,
|
||||
:more_link_for_over_max => triple[3],
|
||||
:title => triple[2],
|
||||
:course_name_counts => course_name_counts }) %>
|
||||
<% end %>
|
||||
</tr>
|
||||
</table>
|
||||
<% unless @current_user.enrollments.ended.empty? && !found_completed %>
|
||||
<div class="menu-item-drop-padded">
|
||||
<%= link_to h(t('menu.all_enrollments', 'See All Enrollments')) + raw(' »'), courses_path, :class => "menu-item-drop-float-right" %>
|
||||
</div>
|
||||
<% end %>
|
||||
</div>
|
||||
<%- else #there is nothing inside the home menu, so dont show it as a drop-down -%>
|
||||
<a href="<%= dashboard_path %>" class="menu-item-no-drop"><%= t 'links.home', 'Home' %></a>
|
||||
<%- if show_home_menu? -%>
|
||||
<%= render(:partial => 'shared/home_menu') %>
|
||||
<%- else -%>
|
||||
<a href="<%= dashboard_path %>" class="menu-item-no-drop"><%= t 'links.home', 'Home' %></a>
|
||||
<%- end -%>
|
||||
</li>
|
||||
<%- end -%>
|
||||
|
|
|
|||
|
|
@ -0,0 +1,12 @@
|
|||
<a href="<%= dashboard_path %>" class="menu-item-title">
|
||||
<%= @current_user.cached_current_group_memberships.empty? ? t('links.courses', "Courses") : t('links.courses_and_groups', "Courses & Groups") -%>
|
||||
<span class="menu-item-title-icon"></span></a>
|
||||
<div class="menu-item-drop">
|
||||
<table cellspacing="0">
|
||||
<tr>
|
||||
<%= render(:partial => "shared/menu_section", :locals => menu_enrollments_locals) %>
|
||||
<%= render(:partial => "shared/menu_section", :locals => menu_groups_locals) %>
|
||||
<%= render(:partial => "shared/menu_section", :locals => menu_accounts_locals) %>
|
||||
</tr>
|
||||
</table>
|
||||
</div>
|
||||
|
|
@ -6,5 +6,5 @@
|
|||
<span class="subtitle ellipsis enrollment_term" style="float: right"><%= menu_enrollment[:enrollment].course.enrollment_term.name %></span>
|
||||
<% end %>
|
||||
<span class="subtitle ellipsis"><%= menu_enrollment[:enrollment].workflow_state == "invited" ? content_tag('b', before_label(:invited_as, 'Invited as')) : before_label(:enrolled_as, "Enrolled as") %> <%= raw(menu_enrollment[:types].sort.map{|t| content_tag('b', t) }.uniq.to_sentence) %></span>
|
||||
</a>
|
||||
</a>
|
||||
</li>
|
||||
|
|
|
|||
|
|
@ -1,13 +1,11 @@
|
|||
<% max_to_show ||= nil; course_name_counts ||= nil %>
|
||||
<% max_to_show ||= nil; course_name_counts ||= nil; link_text ||= nil %>
|
||||
<% if collection && !collection.empty? %>
|
||||
<td class="menu-item-drop-column">
|
||||
<h2><%= title %></h2>
|
||||
<ul class="menu-item-drop-column-list">
|
||||
<%= render :partial => partial, :collection => max_to_show ? collection.first(max_to_show) : collection, :locals => {:title => title, :course_name_counts => course_name_counts} %>
|
||||
<% if collection_size > 0 && collection.size == 0 %>
|
||||
<li><%= link_to("#{collection_size} more", more_link_for_over_max, :class => "menu-item-drop-float-right" ) %></li>
|
||||
<% elsif max_to_show && (collection_size || collection.size) > max_to_show %>
|
||||
<li><%= link_to(raw("… and #{(collection_size || collection.size) - max_to_show} more"), more_link_for_over_max, :class => "menu-item-drop-float-right" ) %></li>
|
||||
<% if link_text %>
|
||||
<li><%= link_to("#{link_text} (#{(collection_size || collection.size)})", more_link_for_over_max, :class => "menu-item-drop-float-right" ) %></li>
|
||||
<% end %>
|
||||
</ul>
|
||||
</td>
|
||||
|
|
|
|||
|
|
@ -21,9 +21,11 @@ require File.expand_path(File.dirname(__FILE__) + '/../spec_helper')
|
|||
describe "enrollment_date_restrictions" do
|
||||
it "should not list inactive enrollments in the menu" do
|
||||
@student = user_with_pseudonym
|
||||
course(:course_name => "Course 1", :active_all => 1)
|
||||
@enrollment1 = course(:course_name => "Course 1", :active_all => 1)
|
||||
e1 = student_in_course(:user => @student, :active_all => 1)
|
||||
course(:course_name => "Course 2", :active_all => 1)
|
||||
|
||||
@enrollment2 = course(:course_name => "Course 2", :active_all => 1)
|
||||
|
||||
@course.update_attributes(:start_at => 2.days.from_now, :conclude_at => 4.days.from_now, :restrict_enrollments_to_course_dates => true)
|
||||
e2 = student_in_course(:user => @student, :active_all => 1)
|
||||
e1.state.should == :active
|
||||
|
|
@ -31,12 +33,14 @@ describe "enrollment_date_restrictions" do
|
|||
e2.state.should == :active
|
||||
e2.state_based_on_date.should == :inactive
|
||||
|
||||
Enrollment.update_all(["created_at = ?", 1.minute.ago]) # need to make created_at and updated_at different
|
||||
|
||||
user_session(@student, @pseudonym)
|
||||
|
||||
get "/"
|
||||
page = Nokogiri::HTML(response.body)
|
||||
list = page.css(".menu-item-drop-column-list li")
|
||||
list.length.should == 1
|
||||
list.length.should == 2 # view all courses should always show up
|
||||
list[0].text.should match /Course 1/
|
||||
list[0].text.should_not match /Course 2/
|
||||
page.css(".menu-item-drop-padded").should be_empty
|
||||
|
|
@ -50,39 +54,6 @@ describe "enrollment_date_restrictions" do
|
|||
page.css(".past_enrollments li").should be_empty
|
||||
end
|
||||
|
||||
it "should include see all enrollments link in menu for date completed courses" do
|
||||
@student = user_with_pseudonym
|
||||
course(:course_name => "Course 1", :active_all => 1)
|
||||
e1 = student_in_course(:user => @student, :active_all => 1)
|
||||
course(:course_name => "Course 2", :active_all => 1)
|
||||
@course.update_attributes(:start_at => 4.days.ago, :conclude_at => 2.days.ago, :restrict_enrollments_to_course_dates => true)
|
||||
e2 = student_in_course(:user => @student, :active_all => 1)
|
||||
e1.state.should == :active
|
||||
e1.state_based_on_date.should == :active
|
||||
e2.state.should == :active
|
||||
e2.state_based_on_date.should == :completed
|
||||
|
||||
user_session(@student, @pseudonym)
|
||||
|
||||
get "/"
|
||||
page = Nokogiri::HTML(response.body)
|
||||
list = page.css(".menu-item-drop-column-list li")
|
||||
list.length.should == 1
|
||||
list[0].text.should match /Course 1/
|
||||
list[0].text.should_not match /Course 2/
|
||||
page.css(".menu-item-drop-padded").should_not be_empty
|
||||
|
||||
get "/courses"
|
||||
page = Nokogiri::HTML(response.body)
|
||||
active_enrollments = page.css(".current_enrollments li")
|
||||
active_enrollments.length.should == 1
|
||||
active_enrollments[0]['class'].should match /active/
|
||||
|
||||
past_enrollments = page.css(".past_enrollments li")
|
||||
past_enrollments.length.should == 1
|
||||
past_enrollments[0]['class'].should match /completed/
|
||||
end
|
||||
|
||||
it "should not show date inactive/completed courses in grades" do
|
||||
@course1 = course(:active_all => 1)
|
||||
@course2 = course(:active_all => 1)
|
||||
|
|
|
|||
|
|
@ -35,11 +35,30 @@ describe "navigation" do
|
|||
get "/"
|
||||
page = Nokogiri::HTML(response.body)
|
||||
list = page.css(".menu-item-drop-column-list li")
|
||||
list[0].text.should match /Summer Term/m # course 3, Summer Term
|
||||
list[1].text.should match /Spring Term/m # course 3, Spring Term
|
||||
|
||||
# order of tests assumes alphabetical order of list
|
||||
list[4].text.should match /Summer Term/m # course 3, Summer Term
|
||||
list[3].text.should match /Spring Term/m # course 3, Spring Term
|
||||
list[2].text.should_not match /Term/ # don't show term cause it doesn't have a name collision
|
||||
list[3].text.should_not match /Term/ # don't show term cause it's the default term
|
||||
list[4].text.should_not match /Term/ # "
|
||||
list[1].text.should_not match /Term/ # don't show term cause it's the default term
|
||||
list[0].text.should_not match /Term/ # "
|
||||
end
|
||||
|
||||
it "should not show enrollments with identical created_at and updated_at dates (bad SIS data)" do
|
||||
@account = Account.default
|
||||
user_with_pseudonym
|
||||
|
||||
course_with_teacher(:course_name => "Course 1", :user => @user)
|
||||
@enrollment = course_with_teacher(:course_name => "Course 2", :user => @user)
|
||||
|
||||
Enrollment.update_all(["updated_at = ?", @enrollment.created_at], :id => @enrollment.id)
|
||||
|
||||
user_session(@user, @pseudonym)
|
||||
|
||||
get "/"
|
||||
page = Nokogiri::HTML(response.body)
|
||||
list = page.css(".menu-item-drop-column-list li")
|
||||
list.length.should == 2
|
||||
end
|
||||
end
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue