ylqjgm
/
canvas-lms
mirror of https://github.com/instructure/canvas-lms.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

favorite courses and groups by default 

test plan:
* courses and groups in the menu should appear by default
 when enrolled or added

closes CNVS-28908

Change-Id: I48885a6748be20e0a010e23a13ad9ad611e04395
Reviewed-on: https://gerrit.instructure.com/80064
Reviewed-by: Jeremy Stanley <jeremy@instructure.com>
QA-Review: Heath Hales <hhales@instructure.com>
Product-Review: Matt Goodwin <mattg@instructure.com>
Tested-by: Jenkins
This commit is contained in:
Alex Boyd 2016-05-18 16:49:33 -06:00 committed by James Williams
parent 9cd1869f77
commit 06cc23f79f
8 changed files with 182 additions and 151 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
app/controllers/application_controller.rb
View File

@ -673,8 +673,8 @@ class ApplicationController < ActionController::Base
groups = group_scope ? group_scope.shard(@context).to_a.reject{|g| g.context_type == "Course" && g.context.concluded?} : []
if opts[:favorites_first]
favorite_course_ids = @context.favorite_context_ids("Course")
courses = courses.sort_by {|c| [favorite_course_ids.include?(c.id) ? 0 : 1, Canvas::ICU.collation_key(c.name)]}
hidden_course_ids = @context.hidden_context_ids("Course")
courses = courses.sort_by {|c| [hidden_course_ids.include?(c.id) ? 1 : 0, Canvas::ICU.collation_key(c.name)]}
end
@contexts.concat courses

102
app/controllers/favorites_controller.rb
View File

@ -44,7 +44,6 @@
class FavoritesController < ApplicationController
before_filter :require_user
before_filter :check_defaults, :only => [:remove_favorite_course]
after_filter :touch_user, :only => [:add_favorite_course, :remove_favorite_course, :reset_course_favorites]
include Api::V1::Favorite
@ -91,7 +90,7 @@ class FavoritesController < ApplicationController
fave_group_memberships = nil
@current_user.shard.activate do
fave_group_memberships = @current_user.groups.active.shard(@current_user).where(id: @current_user.favorite_context_ids("Group"))
fave_group_memberships = @current_user.groups.active.shard(@current_user).where.not(id: @current_user.hidden_context_ids("Group"))
end
if fave_group_memberships.any?
render :json => fave_group_memberships.map{ |g| group_json(g, @current_user,session)}
@ -116,17 +115,7 @@ class FavoritesController < ApplicationController
# -H 'Content-Length: 0'
#
def add_favorite_course
course = api_find(Course, params[:id])
fave = nil
@current_user.shard.activate do
Favorite.unique_constraint_retry do
fave = @current_user.favorites.where(:context_type => 'Course', :context_id => course).first
fave ||= @current_user.favorites.create!(:context => course)
end
end
render :json => favorite_json(fave, @current_user, session)
set_favorite(find_course)
end
# @API Add group to favorites
@ -145,18 +134,8 @@ class FavoritesController < ApplicationController
# -H 'Authorization: Bearer <ACCESS_TOKEN>' \
# -H 'Content-Length: 0'
#
def add_favorite_groups
group = api_find(Group, params[:id])
fave = nil
@current_user.shard.activate do
Favorite.unique_constraint_retry do
fave = @current_user.favorites.where(:context_type => 'Group', :context_id => group).first
fave ||= @current_user.favorites.create!(:context => group)
end
end
render :json => favorite_json(fave, @current_user, session)
def add_favorite_group
set_favorite(find_group)
end
# @API Remove course from favorites
@ -173,21 +152,7 @@ class FavoritesController < ApplicationController
# -H 'Authorization: Bearer <ACCESS_TOKEN>'
#
def remove_favorite_course
# allow removing a Favorite whose context object no longer exists
# but also allow referencing by sis id, if possible
courses = api_find_all(Course, [params[:id]])
course_id = Shard.relative_id_for(courses.any? ? courses.first.id : params[:id], Shard.current, @current_user.shard)
fave = @current_user.favorites.where(:context_type => 'Course', :context_id => course_id).first
if fave
result = favorite_json(fave, @current_user, session)
fave.destroy
render :json => result
else
# can't really return a 404 here without making browsers freak out
# in the Courses UI (it's easy for the client's state to get out of
# sync with the server's, especially with multiple browsers open)
render :json => {}
end
unset_favorite(find_course)
end
# @API Remove group from favorites
@ -203,20 +168,8 @@ class FavoritesController < ApplicationController
# -X DELETE \
# -H 'Authorization: Bearer <ACCESS_TOKEN>'
#
def remove_favorite_groups
group = api_find_all(Group, [params[:id]])
group_id= Shard.relative_id_for(group.any? ? group.first.id : params[:id], Shard.current, @current_user.shard)
fave = @current_user.favorites.where(:context_type => 'Group', :context_id => group_id).first
if fave
result = favorite_json(fave, @current_user, session)
fave.destroy
render :json => result
else
# can't really return a 404 here without making browsers freak out
# in the Group UI (it's easy for the client's state to get out of
# sync with the server's, especially with multiple browsers open)
render :json => {}
end
def remove_favorite_group
unset_favorite(find_group)
end
@ -230,8 +183,8 @@ class FavoritesController < ApplicationController
# -H 'Authorization: Bearer <ACCESS_TOKEN>'
#
def reset_course_favorites
@current_user.favorites.by('Course').destroy_all
render :json => { :status => 'ok' }
Favorite.reset(@current_user, Course)
render json: { status: 'ok' }
end
# @API Reset group favorites
@ -239,29 +192,40 @@ class FavoritesController < ApplicationController
# automatically generated list of enrolled group
#
# @example_request
# curl https://<canvas>/api/v1/users/self/favorites/group \
# curl https://<canvas>/api/v1/users/self/favorites/groups \
# -X DELETE \
# -H 'Authorization: Bearer <ACCESS_TOKEN>'
#
def reset_groups_favorites
@current_user.favorites.by('Group').destroy_all
render :json => { :status => 'ok' }
def reset_group_favorites
Favorite.reset(@current_user, Group)
render json: { status: 'ok' }
end
protected
# When we have other favorites, this needs to be modified to handle the other
# types, rather than just courses.
def check_defaults
return unless @current_user.favorites.count == 0
@current_user.menu_courses.each do |course|
@current_user.favorites.create :context => course
end
end
def touch_user
# Menu is cached, clear it
@current_user.touch
end
def find_course
api_find(Course, params[:id])
end
def find_group
api_find(Group, params[:id])
end
def set_favorite(context)
fave = Favorite.show_context(@current_user, context)
render json: favorite_json(fave, @current_user, session)
end
def unset_favorite(context)
if fave = Favorite.hide_context(@current_user, context)
render json: favorite_json(fave, @current_user, session)
else
render :json => {}
end
end
end

57
app/models/favorite.rb
View File

@ -1,6 +1,59 @@
class Favorite < ActiveRecord::Base
# These are basically now "Unfavorites"
# totes not confusing
belongs_to :context, polymorphic: [:course, :group]
belongs_to :user
validates_inclusion_of :context_type, :allow_nil => true, :in => ['Course', 'Group'].freeze
scope :by, lambda { |type| where(:context_type => type) }
attr_accessible :context, :context_id, :context_type
scope :by, ->(type) { where(:context_type => type.to_s) }
scope :hidden, -> { where(hidden: true) }
scope :not_hidden, -> { where.not(hidden: true) }
attr_accessible :context, :context_id, :context_type, :hidden
# Set whether or not the specified context is considered to be one of the user's favorites. Returns the corresponding
# Favorite object, which may not actually be persisted in the database if it was just deleted or didn't exist.
def self.show_context(user, context)
context_id = Shard.relative_id_for(context.id, Shard.current, user.shard)
fave = user.favorites.where(context_type: context.class.to_s, context_id: context_id).take
if fave
if fave.hidden?
fave.hidden = false
fave.save!
end
fave
else
# Return an anonymous favorite if we didn't actually have one because the old API used to do that
user.shard.activate do
Favorite.new do |f|
f.user = user
f.context = context
f.readonly!
end
end
end
end
# returns nil if it was already hidden
def self.hide_context(user, context)
context_id = Shard.relative_id_for(context.id, Shard.current, user.shard)
Favorite.unique_constraint_retry do
fave = user.favorites.where(context_type: context.class.to_s, context_id: context_id).first
if fave
return nil if fave.hidden? # return nil here if it was already hidden - again because the old API used to do that :/
fave.hidden = true
fave.save!
fave
else
user.favorites.create!(context: context, hidden: true)
end
end
end
def self.reset(user, context_type)
user.favorites.hidden.by(context_type).update_all(:hidden => false)
end
end

36
app/models/user.rb
View File

@ -1666,7 +1666,7 @@ class User < ActiveRecord::Base
@courses_with_primary_enrollment ||= {}
@courses_with_primary_enrollment.fetch(cache_key) do
res = self.shard.activate do
result = Rails.cache.fetch([self, 'courses_with_primary_enrollment2', association, options, ApplicationController.region].cache_key, :expires_in => 15.minutes) do
result = Rails.cache.fetch([self, 'courses_with_primary_enrollment3', association, options, ApplicationController.region].cache_key, :expires_in => 15.minutes) do
# Set the actual association based on if its asking for favorite courses or not.
actual_association = association == :favorite_courses ? :current_and_invited_courses : association
@ -1675,12 +1675,9 @@ class User < ActiveRecord::Base
shards = in_region_associated_shards
# Limit favorite courses based on current shard.
if association == :favorite_courses
ids = self.favorite_context_ids("Course")
if ids.empty?
scope = scope.none
else
shards = shards & ids.map { |id| Shard.shard_for(id) }
scope = scope.where(id: ids)
ids = self.hidden_context_ids("Course")
unless ids.empty?
scope = scope.where.not(id: ids)
end
end
@ -2442,24 +2439,26 @@ class User < ActiveRecord::Base
}
end
# Public: Returns a unique list of favorite context type ids relative to the active shard.
# Public: Returns a unique list of the ids of contexts of the specified type
# that this user has hidden with Favorite.set_favorite(u, c, false). IDs
# will be given relative to the active shard.
#
# Examples
#
# favorite_context_ids("Course")
# hidden_context_ids("Course")
# # => [1, 2, 3, 4]
#
# Returns an array of unique global ids.
def favorite_context_ids(context_type)
@favorite_context_ids ||= {}
# Returns an array of unique relative ids.
def hidden_context_ids(context_type)
@hidden_context_ids ||= {}
context_ids = @favorite_context_ids[context_type]
context_ids = @hidden_context_ids[context_type]
unless context_ids
# Only get the users favorites from their shard.
self.shard.activate do
# Get favorites and map them to their global ids.
context_ids = self.favorites.where(context_type: context_type).pluck(:context_id).map { |id| Shard.global_id_for(id) }
@favorite_context_ids[context_type] = context_ids
context_ids = self.favorites.hidden.where(context_type: context_type).pluck(:context_id).map { |id| Shard.global_id_for(id) }
@hidden_context_ids[context_type] = context_ids
end
end
@ -2472,12 +2471,7 @@ class User < ActiveRecord::Base
def menu_courses(enrollment_uuid = nil)
return @menu_courses if @menu_courses
favorites = self.courses_with_primary_enrollment(:favorite_courses, enrollment_uuid)
if favorites.length > 0
@menu_courses = favorites
else
@menu_courses = self.courses_with_primary_enrollment(:current_and_invited_courses, enrollment_uuid).first(12)
end
@menu_courses = self.courses_with_primary_enrollment(:favorite_courses, enrollment_uuid).first(12)
ActiveRecord::Associations::Preloader.new.preload(@menu_courses, :enrollment_term)
@menu_courses
end

6
config/routes.rb
View File

@ -1453,9 +1453,9 @@ CanvasRails::Application.routes.draw do
delete "users/self/favorites/courses/:id", action: :remove_favorite_course, as: :remove_favorite_course
delete "users/self/favorites/courses", action: :reset_course_favorites
get "users/self/favorites/groups", action: :list_favorite_groups, as: :list_favorite_groups
post "users/self/favorites/groups/:id", action: :add_favorite_groups, as: :add_favorite_groups
delete "users/self/favorites/groups/:id", action: :remove_favorite_groups, as: :remove_favorite_groups
delete "users/self/favorites/groups", action: :reset_groups_favorites
post "users/self/favorites/groups/:id", action: :add_favorite_group, as: :add_favorite_group
delete "users/self/favorites/groups/:id", action: :remove_favorite_group, as: :remove_favorite_group
delete "users/self/favorites/groups", action: :reset_group_favorites
end
scope(controller: :wiki_pages_api) do

7
db/migrate/20160517184312_add_hidden_to_favorites.rb Normal file
View File

@ -0,0 +1,7 @@
class AddHiddenToFavorites < ActiveRecord::Migration
tag :predeploy
def change
add_column :favorites, :hidden, :boolean
end
end

92
spec/apis/v1/favorites_api_spec.rb
View File

@ -87,20 +87,23 @@ describe "Favorites API", type: :request do
context "explicit favorites" do
before :once do
@courses[0...3].each do |course|
@user.favorites.build(:context => course)
@courses[0...2].each do |course|
@user.favorites.build(:context => course) # these basically do nothing now
end
@user.save
end
it "should list favorite courses" do
@courses[3...6].each do |course|
Favorite.hide_context(@user, course)
end
json = api_call(:get, "/api/v1/users/self/favorites/courses", :controller=>"favorites", :action=>"list_favorite_courses", :format=>"json")
expect(json.size).to eql(3)
expect(json[0]['id']).to eql @courses[0].id
expect(json[0]['name']).to eql @courses[0].name
expect(json[0]['course_code']).to eql @courses[0].course_code
expect(json[0]['enrollments'][0]['type']).to eql 'student'
expect(json.collect {|row| row["id"]}.sort).to eql(@user.favorites.by('Course').collect{|c| c[:context_id]}.sort)
expect(json.collect {|row| row["id"]}).to match_array(@courses[0...3].map(&:id))
end
it "should add a course to favorites" do
@ -113,48 +116,54 @@ describe "Favorites API", type: :request do
{:controller=>"favorites", :action=>"add_favorite_course", :format=>"json", :id=>"#{course6.id}"})
expect(json["context_id"]).to eql(course6.id)
# now favorites should include the implicit courses from before, plus the one we faved
# favorites should be empty still because now everything is already a favorite by default
@user.reload
expect(@user.favorites.size).to eql(1)
expect(@user.favorites.size).to eql(0)
end
it "should create favorites from implicit favorites when removing a course" do
it "should remove a course from favorites" do
expect(@user.favorites.size).to eql(2)
# remove a course from favorites
json = api_call(:delete, "/api/v1/users/self/favorites/courses/#{@courses[0].id}",
{:controller=>"favorites", :action=>"remove_favorite_course", :format=>"json", :id=>"#{@courses[0].id}"})
expect(json["context_id"]).to eql(@courses[0].id)
@user.reload
# still shouldn't change the number of favorites
expect(@user.favorites.size).to eql(2)
hidden_fav = @user.favorites.detect{|f| f.context == @courses[0]}
expect(hidden_fav).to be_hidden
end
it "should remove an implicitly favorited course from favorites" do
@user.favorites.by("Course").destroy_all
# remove a course from favorites
json = api_call(:delete, "/api/v1/users/self/favorites/courses/#{@courses[0].id}",
{:controller=>"favorites", :action=>"remove_favorite_course", :format=>"json", :id=>"#{@courses[0].id}"})
{:controller=>"favorites", :action=>"remove_favorite_course", :format=>"json", :id=>"#{@courses[0].id}"})
expect(json["context_id"]).to eql(@courses[0].id)
# now favorites should include the implicit courses from before, minus the one we removed
@user.reload
expect(@user.favorites.size).to eql(5)
end
it "should remove a course from favorites" do
expect(@user.favorites.size).to eql(3)
# remove a course from favorites
json = api_call(:delete, "/api/v1/users/self/favorites/courses/#{@courses[0].id}",
{:controller=>"favorites", :action=>"remove_favorite_course", :format=>"json", :id=>"#{@courses[0].id}"})
expect(json["context_id"]).to eql(@courses[0].id)
# now favorites should include the implicit courses from before, minus the one we removed
@user.reload
expect(@user.favorites.size).to eql(2)
# should create a hidden "favorite"
expect(@user.favorites.size).to eql(1)
hidden_fav = @user.favorites.detect{|f| f.context == @courses[0]}
expect(hidden_fav).to be_hidden
end
it "should not create a duplicate by fav'ing an already faved course" do
expect(@user.favorites.size).to eql(3)
expect(@user.favorites.size).to eql(2)
json = api_call(:post, "/api/v1/users/self/favorites/courses/#{@courses[0].id}",
{:controller=>"favorites", :action=>"add_favorite_course", :format=>"json", :id=>"#{@courses[0].id}"})
expect(json["context_id"]).to eql(@courses[0].id)
@user.reload
expect(@user.favorites.size).to eql(3)
expect(@user.favorites.size).to eql(2)
end
it "should return an empty hash when removing a non-faved course" do
Favorite.hide_context(@user, @courses[5])
json = api_call(:delete, "/api/v1/users/self/favorites/courses/#{@courses[5].id}",
{:controller=>"favorites", :action=>"remove_favorite_course", :format=>"json", :id=>"#{@courses[5].id}"})
expect(json.size).to be_zero
@ -165,7 +174,7 @@ describe "Favorites API", type: :request do
api_call(:delete, "/api/v1/users/self/favorites/courses",
{:controller=>"favorites", :action=>"reset_course_favorites", :format=>"json"})
@user.reload
expect(@user.favorites.size).to be_zero
expect(@user.favorites.hidden.count).to be_zero
end
end
@ -184,39 +193,36 @@ describe "Favorites API", type: :request do
end
it "add favorite group" do
api_call(:post, "/api/v1/users/self/favorites/groups/#{@group_not_yet_fave.id}",
:controller=>"favorites", :action=>"add_favorite_groups", :format=>"json", :id=>@group_not_yet_fave.id)
expect(@user.favorites.size).to eql(2) # Added one in before method, one in test
:controller=>"favorites", :action=>"add_favorite_group", :format=>"json", :id=>@group_not_yet_fave.id)
expect(@user.favorites.size).to eql(1) # already implicitly favorited
end
it "lists favorite groups" do
Favorite.hide_context(@user, @group_not_fave)
Favorite.hide_context(@user, @group_not_yet_fave)
json = api_call(:get, "/api/v1/users/self/favorites/groups",
:controller=>"favorites", :action=>"list_favorite_groups", :format=>"json")
expect(json.size).to eq 1
expect(json[0]['id']).to eql @group_fave.id
expect(@user.favorites.size).to eql(1)
end
it "clears favorite groups" do
expect(@user.favorites.size).to eql(1)
group_fave_2 = Group.create!(:name=>"new_fave", :context=>@context)
group_fave_2.add_user(@user)
api_call(:post, "/api/v1/users/self/favorites/groups/#{group_fave_2.id}",
:controller=>"favorites", :action=>"add_favorite_groups", :format=>"json", :id=>group_fave_2.id)
expect(@user.favorites.size).to eql(2)
api_call(:delete, "/api/v1/users/self/favorites/groups/#{group_fave_2.id}",
:controller=>"favorites", :action=>"remove_favorite_group", :format=>"json", :id=>group_fave_2.id)
expect(@user.favorites.hidden.size).to eql(1)
api_call(:delete, "/api/v1/users/self/favorites/groups",
:controller=>"favorites", :action=>"reset_groups_favorites", :format=>"json")
expect(@user.favorites.size).to eql(0)
:controller=>"favorites", :action=>"reset_group_favorites", :format=>"json")
expect(@user.favorites.hidden.size).to eql(0)
end
it "deletes one favorite group" do
json = api_call(:delete, "/api/v1/users/self/favorites/groups/#{@group_fave.id}",
:controller=>"favorites", :action=>"remove_favorite_groups", :format=>"json", :id=>@group_fave.id)
:controller=>"favorites", :action=>"remove_favorite_group", :format=>"json", :id=>@group_fave.id)
expect(json['context_type']).to eql("Group")
end
it "lists all groups if none are favorited" do
api_call(:delete, "/api/v1/users/self/favorites/groups",
:controller=>"favorites", :action=>"reset_groups_favorites", :format=>"json")
expect(@user.favorites.size).to eql(0)
json = api_call(:get, "/api/v1/users/self/favorites/groups",
:controller=>"favorites", :action=>"list_favorite_groups", :format=>"json")
expect(json.any?)
end
end
end

29
spec/models/user_spec.rb
View File

@ -1330,7 +1330,8 @@ describe User do
course(:active_all => 1)
@course.enroll_user(@user2)
expect(@user1.menu_courses).to eq [@course]
expect(@user1.menu_courses).to eq []
expect(@user2.menu_courses).to eq [@course]
end
end
@ -1342,20 +1343,18 @@ describe User do
(1..3).each do |x|
course = course_with_student(:course_name => "Course #{x}", :user => @user, :active_all => true).course
@courses << course
@user.favorites.create!(context: course)
end
@user.save!
end
it "should default favorites to enrolled courses when favorite courses do not exist" do
@user.favorites.by("Course").destroy_all
it "should default favorites to enrolled courses" do
expect(@user.menu_courses.to_set).to eq @courses.to_set
end
it "should only include favorite courses when set" do
course = @courses.shift
@user.favorites.where(context_type: "Course", context_id: course).first.destroy
it "should only include favorite courses if some have been hidden" do
hidden_course = @courses.shift
Favorite.hide_context(@user, hidden_course)
expect(@user.menu_courses.to_set).to eq @courses.to_set
end
@ -1364,16 +1363,24 @@ describe User do
before :each do
account2 = @shard1.activate { account_model }
@cs_courses = []
(4..6).each do |x|
course = course_with_student(:course_name => "Course #{x}", :user => @user, :active_all => true, :account => account2).course
@courses << course
@user.favorites.create!(context: course)
@cs_courses << course
end
end
it "should include cross shard favorite courses" do
@user.favorites.by("Course").where("id % 2 = 0").destroy_all
expect(@user.menu_courses.size).to eql(@courses.length / 2)
hidden_course = @courses.shift
Favorite.hide_context(@user, hidden_course)
hidden_cs_course = @cs_courses.shift
Favorite.hide_context(@user, hidden_cs_course)
menu_courses = @user.menu_courses
expect(menu_courses.count).to eq 4
expect(menu_courses).to_not include(hidden_course)
expect(menu_courses).to_not include(hidden_cs_course)
end
end
end