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

items and followers counts for collections 

aggregate and cache the number of items and number of followers of
a collection. return these counts in the api response.

test plan:
- run the migration
- query existing collections and make sure counts look accurate
- follow/unfollow collections and make sure count changes
- pin/delete items and make sure the count changes

Change-Id: Ic3a96682b550c168f505f419ffc4b619f859f044
Reviewed-on: https://gerrit.instructure.com/11490
Tested-by: Jenkins <jenkins@instructure.com>
Reviewed-by: Brian Palmer <brianp@instructure.com>
This commit is contained in:
Simon Williams 2012-06-07 14:53:24 -06:00
parent ebcaec662c
commit 79e4301861
11 changed files with 301 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
app/controllers/collections_controller.rb
View File

@ -46,7 +46,13 @@
# visibility: "public",
#
# // Boolean indicating whether this user is following this collection.
# followed_by_user: false
# followed_by_user: false,
#
# // The number of people following this collection.
# followers_count: 10,
#
# // The number of items in this collection.
# items_count: 7
# }
#
class CollectionsController < ApplicationController

30
app/models/collection_item.rb
View File

@ -91,6 +91,36 @@ class CollectionItem < ActiveRecord::Base
end
end
trigger.after(:insert) do |t|
t.where("NEW.workflow_state = 'active'") do
<<-SQL
UPDATE collections
SET items_count = items_count + 1
WHERE id = NEW.collection_id;
SQL
end
end
trigger.after(:update) do |t|
t.where("NEW.workflow_state <> OLD.workflow_state") do
<<-SQL
UPDATE collections
SET items_count = items_count + CASE WHEN (NEW.workflow_state = 'active') THEN 1 ELSE -1 END
WHERE id = NEW.collection_id;
SQL
end
end
trigger.after(:delete) do |t|
t.where("OLD.workflow_state = 'active'") do
<<-SQL
UPDATE collections
SET items_count = items_count - 1
WHERE id = OLD.collection_id;
SQL
end
end
set_policy do
given { |user, session| self.collection.grants_right?(user, session, :read) }
can :read

20
app/models/user_follow.rb
View File

@ -128,6 +128,26 @@ class UserFollow < ActiveRecord::Base
end
end
trigger.after(:insert) do |t|
t.where("NEW.followed_item_type = 'Collection'") do
<<-SQL
UPDATE collections
SET followers_count = followers_count + 1
WHERE id = NEW.followed_item_id;
SQL
end
end
trigger.after(:delete) do |t|
t.where("OLD.followed_item_type = 'Collection'") do
<<-SQL
UPDATE collections
SET followers_count = followers_count - 1
WHERE id = OLD.followed_item_id;
SQL
end
end
# returns the subset of items that are currently being followed by the given user
#
# currently this method assumes that all the items are of the same type, this

13
db/migrate/20120607195540_add_aggregate_counts_to_collections.rb Normal file
View File

@ -0,0 +1,13 @@
class AddAggregateCountsToCollections < ActiveRecord::Migration
tag :predeploy
def self.up
add_column :collections, :followers_count, :integer, :default => 0
add_column :collections, :items_count, :integer, :default => 0
end
def self.down
remove_column :collections, :followers_count
remove_column :collections, :items_count
end
end

91
db/migrate/20120608165313_create_collection_items_count_and_followers_count_triggers.rb Normal file
View File

@ -0,0 +1,91 @@
# This migration was auto-generated via `rake db:generate_trigger_migration'.
# While you can edit this file, any changes you make to the definitions here
# will be undone by the next auto-generated trigger migration.
class CreateCollectionItemsCountAndFollowersCountTriggers < ActiveRecord::Migration
tag :predeploy
def self.up
create_trigger("collection_items_after_insert_row_tr", :generated => true, :compatibility => 1).
on("collection_items").
after(:insert) do |t|
t.where("NEW.workflow_state = 'active'") do
<<-SQL_ACTIONS
UPDATE collections
SET items_count = items_count + 1
WHERE id = NEW.collection_id;
SQL_ACTIONS
end
end
create_trigger("collection_items_after_update_row_tr", :generated => true, :compatibility => 1).
on("collection_items").
after(:update) do |t|
t.where("NEW.workflow_state <> OLD.workflow_state") do
<<-SQL_ACTIONS
UPDATE collections
SET items_count = items_count + CASE WHEN (NEW.workflow_state = 'active') THEN 1 ELSE -1 END
WHERE id = NEW.collection_id;
SQL_ACTIONS
end
end
create_trigger("collection_items_after_delete_row_tr", :generated => true, :compatibility => 1).
on("collection_items").
after(:delete) do |t|
t.where("OLD.workflow_state = 'active'") do
<<-SQL_ACTIONS
UPDATE collections
SET items_count = items_count - 1
WHERE id = OLD.collection_id;
SQL_ACTIONS
end
end
create_trigger("user_follows_after_insert_row_tr", :generated => true, :compatibility => 1).
on("user_follows").
after(:insert) do |t|
t.where("NEW.followed_item_type = 'Collection'") do
<<-SQL_ACTIONS
UPDATE collections
SET followers_count = followers_count + 1
WHERE id = NEW.followed_item_id;
SQL_ACTIONS
end
end
create_trigger("user_follows_after_delete_row_tr", :generated => true, :compatibility => 1).
on("user_follows").
after(:delete) do |t|
t.where("OLD.followed_item_type = 'Collection'") do
<<-SQL_ACTIONS
UPDATE collections
SET followers_count = followers_count - 1
WHERE id = OLD.followed_item_id;
SQL_ACTIONS
end
end
end
def self.down
drop_trigger("collection_items_after_insert_row_tr", "collection_items", :generated => true)
drop_trigger("collection_items_after_insert_row_when_new_workflow_state_ac_tr", "collection_items", :generated => true)
drop_trigger("collection_items_after_update_row_tr", "collection_items", :generated => true)
drop_trigger("collection_items_after_update_row_when_new_workflow_state_ol_tr", "collection_items", :generated => true)
drop_trigger("collection_items_after_delete_row_tr", "collection_items", :generated => true)
drop_trigger("collection_items_after_delete_row_when_old_workflow_state_ac_tr", "collection_items", :generated => true)
drop_trigger("user_follows_after_insert_row_tr", "user_follows", :generated => true)
drop_trigger("user_follows_after_insert_row_when_new_followed_item_type_co_tr", "user_follows", :generated => true)
drop_trigger("user_follows_after_delete_row_tr", "user_follows", :generated => true)
drop_trigger("user_follows_after_delete_row_when_old_followed_item_type_co_tr", "user_follows", :generated => true)
end
end

10
db/migrate/20120608165707_count_existing_collection_items_and_followers.rb Normal file
View File

@ -0,0 +1,10 @@
class CountExistingCollectionItemsAndFollowers < ActiveRecord::Migration
tag :postdeploy
def self.up
DataFixup::CountExistingCollectionItemsAndFollowers.send_later_if_production(:run)
end
def self.down
end
end

2
lib/api/v1/collection.rb
View File

@ -21,7 +21,7 @@ module Api::V1::Collection
include Api::V1::User
API_COLLECTION_JSON_OPTS = {
:only => %w(id name visibility),
:only => %w(id name visibility followers_count items_count),
}
API_COLLECTION_ITEM_JSON_OPTS = {

20
lib/data_fixup/count_existing_collection_items_and_followers.rb Normal file
View File

@ -0,0 +1,20 @@
module DataFixup::CountExistingCollectionItemsAndFollowers
def self.run
Collection.active.find_ids_in_batches do |ids|
Collection.connection.execute(Collection.send(:sanitize_sql_array, [<<-SQL, ids]))
UPDATE collections c SET followers_count = (
SELECT COUNT(*)
FROM user_follows uf
WHERE uf.followed_item_id = c.id
AND uf.followed_item_type = 'Collection'
), items_count = (
SELECT COUNT(*)
FROM collection_items ci
WHERE ci.workflow_state = 'active'
AND ci.collection_id = c.id
)
WHERE id IN (?)
SQL
end
end
end

46
spec/apis/v1/collections_spec.rb
View File

@ -23,12 +23,12 @@ describe "Collections API", :type => :integration do
it "should allow retrieving a paginated collection list" do
json = api_call(:get, @collections_path, @collections_path_options)
response['Link'].should be_present
json.should == [ @c2_json, @c1_json ]
json.should == [ collection_json(@c2), collection_json(@c1) ]
end
it "should allow retrieving a private collection" do
json = api_call(:get, "/api/v1/collections/#{@c1.id}", { :controller => "collections", :collection_id => @c1.to_param, :action => "show", :format => "json" })
json.should == @c1_json
json.should == collection_json(@c1)
end
it "should allow creating a collection" do
@ -37,19 +37,18 @@ describe "Collections API", :type => :integration do
:visibility => 'public',
})
@c3 = Collection.last(:order => :id)
json.should == {
json.should == collection_json(@c3).merge({
'id' => @c3.id,
'name' => 'test3',
'visibility' => 'public',
'followed_by_user' => false,
}
})
end
it "should allow updating a collection" do
json = api_call(:put, "/api/v1/collections/#{@c1.id}", { :controller => "collections", :collection_id => @c1.to_param, :action => "update", :format => "json" }, {
:name => "test1 edited",
})
json.should == @c1_json.merge('name' => 'test1 edited')
json.should == collection_json(@c1).merge('name' => 'test1 edited')
@c1.reload.name.should == "test1 edited"
end
@ -82,7 +81,7 @@ describe "Collections API", :type => :integration do
it "should not return in list" do
json = api_call(:get, @collections_path, @collections_path_options)
json.should == [ @c2_json ]
json.should == [ collection_json(@c2) ]
end
it "should not allow getting" do
@ -99,12 +98,12 @@ describe "Collections API", :type => :integration do
it "should only list public collections" do
json = api_call(:get, @collections_path, @collections_path_options)
response['Link'].should be_present
json.should == [ @c2_json ]
json.should == [ collection_json(@c2) ]
end
it "should allow getting a public collection" do
json = api_call(:get, "/api/v1/collections/#{@c2.id}", { :controller => "collections", :collection_id => @c2.to_param, :action => "show", :format => "json" })
json.should == @c2_json
json.should == collection_json(@c2)
end
it "should not allow getting a private collection" do
@ -137,20 +136,6 @@ describe "Collections API", :type => :integration do
def create_collections(context)
@c1 = context.collections.create!(:name => 'test1', :visibility => 'private')
@c2 = context.collections.create!(:name => 'test2', :visibility => 'public')
@c1_json =
{
'id' => @c1.id,
'name' => @c1.name,
'visibility' => 'private',
'followed_by_user' => false,
}
@c2_json =
{
'id' => @c2.id,
'name' => @c2.name,
'visibility' => 'public',
'followed_by_user' => false,
}
end
def create_collection_items(user)
@ -165,6 +150,17 @@ describe "Collections API", :type => :integration do
@c2_items_path_options = { :controller => "collection_items", :action => "index", :format => "json", :collection_id => @c2.to_param }
end
def collection_json(collection)
{
'id' => collection.id,
'name' => collection.name,
'visibility' => collection.visibility,
'followed_by_user' => false,
'followers_count' => collection.followers.count,
'items_count' => collection.collection_items.active.count,
}
end
def item_json(item, upvoted_by_user = false)
{
'id' => item.id,
@ -504,7 +500,7 @@ describe "Collections API", :type => :integration do
it "should allow retrieving a paginated collection list" do
json = api_call(:get, @collections_path, @collections_path_options)
response['Link'].should be_present
json.should == [ @c2_json, @c1_json ]
json.should == [ collection_json(@c2), collection_json(@c1) ]
end
it "should create a default private collection if no collections exist for the context" do
@ -518,7 +514,7 @@ describe "Collections API", :type => :integration do
it "should allow retrieving a private collection" do
json = api_call(:get, "/api/v1/collections/#{@c1.id}", { :controller => "collections", :collection_id => @c1.to_param, :action => "show", :format => "json" })
json.should == @c1_json
json.should == collection_json(@c1)
end
it "should not allow creating a collection" do

48
spec/migrations/count_existing_collection_items_and_followers_spec.rb Normal file
View File

@ -0,0 +1,48 @@
#
# Copyright (C) 2012 Instructure, Inc.
#
# This file is part of Canvas.
#
# Canvas is free software: you can redistribute it and/or modify it under
# the terms of the GNU Affero General Public License as published by the Free
# Software Foundation, version 3 of the License.
#
# Canvas is distributed in the hope that it will be useful, but WITHOUT ANY
# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
# A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
# details.
#
# You should have received a copy of the GNU Affero General Public License along
# with this program. If not, see <http://www.gnu.org/licenses/>.
#
require File.expand_path(File.dirname(__FILE__) + '/../spec_helper.rb')
describe "CountExistingCollectionItemsAndFollowers" do
it "works" do
u1 = user_model
u2 = user_model
u3 = user_model
c1 = u1.collections.create!(:name => 'c1', :visibility => 'public')
c2 = u1.collections.create!(:name => 'c2', :visibility => 'private')
c3 = u2.collections.create!(:name => 'c3', :visibility => 'public')
c4 = u3.collections.create!(:name => 'c4', :visibility => 'public')
c4.destroy
i11 = collection_item_model(:user_comment => "item 1", :user => u1, :collection => c1, :collection_item_data => collection_item_data_model(:link_url => "http://www.example.com/one"))
i12 = collection_item_model(:user_comment => "item 2", :user => u1, :collection => c1, :collection_item_data => collection_item_data_model(:link_url => "http://www.example.com/two"))
i21 = collection_item_model(:user_comment => "item 1 private", :user => u1, :collection => c2, :collection_item_data => collection_item_data_model(:link_url => "http://www.example.com/two"))
UserFollow.create_follow(u2, c1)
UserFollow.create_follow(u3, c3)
UserFollow.create_follow(u1, c3)
DataFixup::CountExistingCollectionItemsAndFollowers.run
cs = [c1,c2,c3,c4]
cs.map(&:reload)
cs.map(&:followers_count).should == [1,0,2,0]
cs.map(&:items_count).should == [2,1,0,0]
end
end

41
spec/models/collections_spec.rb
View File

@ -20,12 +20,15 @@ require File.expand_path(File.dirname(__FILE__) + '/../spec_helper')
describe 'Collections' do
shared_examples_for "auto-follow context" do
it "should auto-follow for users following" do
def setup_collections
@pub1 = @context.collections.create!(:visibility => "public")
@pri1 = @context.collections.create!(:visibility => "private")
@del1 = @context.collections.create!(:visibility => "public")
run_jobs
end
it "should auto-follow for users following" do
setup_collections
@del1.destroy
UserFollow.create_follow(@user, @context)
run_jobs
@ -46,6 +49,42 @@ describe 'Collections' do
@pub2.destroy
@pub2.reload.followers.should == []
end
it "should correctly calculate followers_count" do
setup_collections
@del1.destroy
5.times { UserFollow.create_follow(user_model, @context) }
run_jobs
@pub1.reload.followers_count.should == @pub1.reload.followers.count
@pri1.reload.followers_count.should == (@follows_private ? @pri1.reload.followers.count : 0)
@del1.reload.followers_count.should == 0
UserFollow.destroy_all
@pub1.reload.followers_count.should == 0
@pri1.reload.followers_count.should == 0
end
it "should correctly calculate items_count" do
setup_collections
5.times do
[@pub1, @pri1, @del1].each do |col|
collection_item_model(:user_comment => "item 1",
:user => @user,
:collection => col,
:collection_item_data => collection_item_data_model(:link_url => "http://www.example.com/one"))
end
end
@del1.destroy
@pub1.reload.items_count.should == 5
@pri1.reload.items_count.should == 5
@del1.reload.items_count.should == 5
@del1.collection_items.destroy_all
@del1.reload.items_count.should == 0
end
end
describe "user collections" do