Skip to content

C
cs_comments_service
Commit 1f26217d by Robert Raposa
Options

Remove sort order.

parent bb116a19
Showing with 52 additions and 128 deletions
api/comment_threads.rb
......@@ -13,7 +13,6 @@ get "#{APIPREFIX}/threads" do # retrieve threads by course
value_to_boolean(params["unread"]),
value_to_boolean(params["unanswered"]),
params["sort_key"],
params["sort_order"],
params["page"],
params["per_page"]
).to_json
......
api/commentables.rb
......@@ -18,7 +18,6 @@ get "#{APIPREFIX}/:commentable_id/threads" do |commentable_id|
value_to_boolean(params["unread"]),
value_to_boolean(params["unanswered"]),
params["sort_key"],
params["sort_order"],
params["page"],
params["per_page"],
params["context"] ? params["context"] : :course
......
api/notifications_and_subscriptions.rb
......@@ -12,7 +12,6 @@ get "#{APIPREFIX}/users/:user_id/subscribed_threads" do |user_id|
value_to_boolean(params["unread"]),
value_to_boolean(params["unanswered"]),
params["sort_key"],
params["sort_order"],
params["page"],
params["per_page"]
).to_json
......
api/search.rb
......@@ -88,7 +88,6 @@ get "#{APIPREFIX}/search/threads" do
value_to_boolean(local_params["unread"]),
value_to_boolean(local_params["unanswered"]),
local_params["sort_key"],
local_params["sort_order"],
local_params["page"],
local_params["per_page"],
context
......
lib/helpers.rb
......@@ -137,7 +137,6 @@ helpers do
filter_unread,
filter_unanswered,
sort_key,
sort_order,
page,
per_page,
context=:course
......@@ -175,7 +174,7 @@ helpers do
end
end
sort_criteria = get_sort_criteria(sort_key, sort_order)
sort_criteria = get_sort_criteria(sort_key)
if not sort_criteria
{}
else
......@@ -240,7 +239,7 @@ helpers do
# Given query params, return sort criteria appropriate for passing to the
# order_by function of a Mongoid query. Returns nil if params are not valid.
def get_sort_criteria(sort_key, sort_order)
def get_sort_criteria(sort_key)
sort_key_mapper = {
"date" => :created_at,
"activity" => :last_activity_at,
......@@ -248,16 +247,11 @@ helpers do
"comments" => :comment_count,
}
sort_order_mapper = {
"desc" => :desc,
"asc" => :asc,
}
sort_key = sort_key_mapper[params["sort_key"] || "date"]
sort_order = sort_order_mapper[params["sort_order"] || "desc"]
if sort_key && sort_order
sort_criteria = [[:pinned, :desc], [sort_key, sort_order]]
if sort_key
# only sort order of :desc is supported. support for :asc would require new indices.
sort_criteria = [[:pinned, :desc], [sort_key, :desc]]
if ![:created_at, :last_activity_at].include? sort_key
sort_criteria << [:created_at, :desc]
end
......
lib/tasks/benchmark.rake
......@@ -76,19 +76,18 @@ namespace :benchmark do
Benchmark.bm(31) do |x|
sort_keys = %w[date activity votes comments]
sort_order = "desc"
x.report("querying threads in a course") do
(1..COURSE_THREAD_QUERY).each do |seed|
query_params = { course_id: "1", sort_key: sort_keys[seed % 4], sort_order: sort_order, page: seed % 5 + 1, per_page: 5 }
query_params = { course_id: "1", sort_key: sort_keys[seed % 4], page: seed % 5 + 1, per_page: 5 }
RestClient.get "#{PREFIX}/threads", params: query_params
end
end
x.report("searching threads in a course") do
(1..COURSE_THREAD_QUERY).each do |seed|
query_params = { course_id: "1", text: "token#{seed % 10} token#{(seed * seed) % 10}", sort_key: sort_keys[seed % 4], sort_order: sort_order, page: seed % 5 + 1, per_page: 5 }
query_params = { course_id: "1", text: "token#{seed % 10} token#{(seed * seed) % 10}", sort_key: sort_keys[seed % 4], page: seed % 5 + 1, per_page: 5 }
RestClient.get "#{PREFIX}/search/threads", params: query_params
end
end
......
lib/tasks/deep_search.rake
......@@ -50,7 +50,6 @@ namespace :deep_search do
end
sort_keys = %w[date activity votes comments]
sort_order = "desc"
#set the sinatra env to test to avoid 401'ing
set :environment, :test
......@@ -58,7 +57,7 @@ namespace :deep_search do
start_time = Time.now
puts "Starting test at #{start_time}"
1000.times do |i|
query_params = { course_id: "1", sort_key: sort_keys.sample, sort_order: sort_order, page: 1, per_page: 5, text: bodies.sample }
query_params = { course_id: "1", sort_key: sort_keys.sample, page: 1, per_page: 5, text: bodies.sample }
RestClient.get "#{PREFIX}/threads", params: query_params
end
end_time = Time.now
......
spec/api/comment_thread_spec.rb
......@@ -22,7 +22,7 @@ describe "app" do
t.course_id = "abc"
t.save!
end
rs = thread_result course_id: "abc", sort_order: "asc"
rs = thread_result course_id: "abc"
rs.length.should == 2
rs.each_with_index { |res, i|
check_thread_result_json(nil, @threads["t#{i+1}"], res)
......@@ -36,7 +36,7 @@ describe "app" do
end
@threads["t2"].context = :standalone
@threads["t2"].save!
rs = thread_result course_id: "abc", sort_order: "asc"
rs = thread_result course_id: "abc"
rs.length.should == 2
check_thread_result_json(nil, @threads["t1"], rs[0])
check_thread_result_json(nil, @threads["t3"], rs[1])
......@@ -77,7 +77,7 @@ describe "app" do
@threads["t2"].course_id = "omg"
@threads["t2"].group_id = 101
@threads["t2"].save!
rs = thread_result course_id: "omg", group_ids: "100,101", sort_order: "asc"
rs = thread_result course_id: "omg", group_ids: "100,101"
rs.length.should == 2
end
it "returns only threads where course id and group id match or group id is nil" do
......@@ -88,7 +88,7 @@ describe "app" do
@threads["t2"].save!
@threads["t3"].group_id = 100
@threads["t3"].save!
rs = thread_result course_id: "omg", group_id: 100, sort_order: "asc"
rs = thread_result course_id: "omg", group_id: 100
rs.length.should == 2
rs.each_with_index { |res, i|
check_thread_result_json(nil, @threads["t#{i+1}"], res)
......@@ -184,7 +184,7 @@ describe "app" do
t.course_id = "abc"
t.save!
end
rs = thread_result course_id: "abc", user_id: "123", sort_order: "asc"
rs = thread_result course_id: "abc", user_id: "123"
rs.length.should == 2
rs.each_with_index { |result, i|
check_thread_result_json(user, @threads["t#{i+1}"], result)
......@@ -194,7 +194,7 @@ describe "app" do
}
user.mark_as_read(@threads["t1"])
rs = thread_result course_id: "abc", user_id: "123", sort_order: "asc"
rs = thread_result course_id: "abc", user_id: "123"
rs.length.should == 2
rs.each_with_index { |result, i|
check_thread_result_json(user, @threads["t#{i+1}"], result)
......@@ -206,7 +206,7 @@ describe "app" do
@threads["t1"].updated_at += 1 # 1 second later
@threads["t1"].save!
rs = thread_result course_id: "abc", user_id: "123", sort_order: "asc"
rs = thread_result course_id: "abc", user_id: "123"
rs.length.should == 2
rs.each_with_index { |result, i|
check_thread_result_json(user, @threads["t#{i+1}"], result)
......@@ -218,18 +218,18 @@ describe "app" do
# author's own posts should not count as unread
make_comment(user, @threads["t1"], "my two cents")
rs = thread_result course_id: "abc", user_id: "123", sort_order: "asc"
rs = thread_result course_id: "abc", user_id: "123"
rs[0]["unread_comments_count"].should == 0
# other's posts do, though
make_comment(@threads["t1"].author, @threads["t1"], "the last word")
rs = thread_result course_id: "abc", user_id: "123", sort_order: "asc"
rs = thread_result course_id: "abc", user_id: "123"
rs[0]["unread_comments_count"].should == 1
end
context "sorting" do
def thread_result_order (sort_key, sort_order)
results = thread_result course_id: DFLT_COURSE_ID, sort_key: sort_key, sort_order: sort_order
def thread_result_order (sort_key)
results = thread_result course_id: DFLT_COURSE_ID, sort_key: sort_key
results.length.should == 10
results.map { |t| t["title"] }
end
......@@ -248,13 +248,8 @@ describe "app" do
ary
end
it "sorts using create date / ascending" do
actual_order = thread_result_order("date", "asc")
expected_order = @default_order.reverse
actual_order.should == expected_order
end
it "sorts using create date / descending" do
actual_order = thread_result_order("date", "desc")
actual_order = thread_result_order("date")
expected_order = @default_order
actual_order.should == expected_order
end
......@@ -262,141 +257,92 @@ describe "app" do
t5 = @threads["t5"]
t5.update(body: "changed!")
t5.save!
actual_order = thread_result_order("activity", "desc")
actual_order = thread_result_order("activity")
expected_order = @default_order
actual_order.should == expected_order
end
it "sort unchanged using last activity / ascending when thread is updated" do
t5 = @threads["t5"]
t5.update(body: "changed!")
t5.save!
actual_order = thread_result_order("activity", "asc")
expected_order = @default_order.reverse
actual_order.should == expected_order
end
it "sort unchanged using last activity / descending when comment is updated" do
t5c = @threads["t5"].comments.first
t5c.update(body: "changed!")
t5c.save!
actual_order = thread_result_order("activity", "desc")
actual_order = thread_result_order("activity")
expected_order = @default_order
actual_order.should == expected_order
end
it "sort unchanged using last activity / ascending when comment is updated" do
t5c = @threads["t5"].comments.first
t5c.update(body: "changed!")
t5c.save!
actual_order = thread_result_order("activity", "asc")
expected_order = @default_order.reverse
actual_order.should == expected_order
end
it "sorts using last activity / descending when response is created" do
t5 = @threads["t5"]
comment = t5.comments.new(body: "this problem is so easy", course_id: "1")
comment.author = User.first
comment.save!
actual_order = thread_result_order("activity", "desc")
actual_order = thread_result_order("activity")
expected_order = move_to_front(@default_order, "t5")
actual_order.should == expected_order
end
it "sorts using last activity / ascending when response is created" do
t5 = @threads["t5"]
comment = t5.comments.new(body: "this problem is so easy", course_id: "1")
comment.author = User.first
comment.save!
actual_order = thread_result_order("activity", "asc")
expected_order = move_to_end(@default_order.reverse, "t5")
actual_order.should == expected_order
end
it "sorts using vote count / descending" do
user = User.all.first
t5 = @threads["t5"]
user.vote(t5, :up)
t5.save!
actual_order = thread_result_order("votes", "desc")
actual_order = thread_result_order("votes")
expected_order = move_to_front(@default_order, "t5")
actual_order.should == expected_order
end
it "sorts using vote count / ascending" do
user = User.all.first
t5 = @threads["t5"]
user.vote(t5, :up)
t5.save!
actual_order = thread_result_order("votes", "asc")
expected_order = move_to_end(@default_order, "t5")
actual_order.should == expected_order
end
it "sorts using comment count / descending" do
make_comment(@threads["t5"].author, @threads["t5"], "extra comment")
actual_order = thread_result_order("comments", "desc")
actual_order = thread_result_order("comments")
expected_order = move_to_front(@default_order, "t5")
actual_order.should == expected_order
end
it "sorts using comment count / ascending" do
make_comment(@threads["t5"].author, @threads["t5"], "extra comment")
actual_order = thread_result_order("comments", "asc")
expected_order = move_to_end(@default_order, "t5")
actual_order.should == expected_order
end
it "sorts pinned items first" do
make_comment(@threads["t5"].author, @threads["t5"], "extra comment")
@threads["t7"].pinned = true
@threads["t7"].save!
actual_order = thread_result_order("comments", "asc")
expected_order = move_to_front(move_to_end(@default_order, "t5"), "t7")
actual_order.should == expected_order
actual_order = thread_result_order("comments", "desc")
actual_order = thread_result_order("comments")
expected_order = move_to_front(move_to_front(@default_order, "t5"), "t7")
actual_order.should == expected_order
@threads["t8"].pinned = true
@threads["t8"].save!
actual_order = thread_result_order("comments", "asc")
expected_order = move_to_front(move_to_end(@default_order, "t5"), "t8", "t7")
actual_order = thread_result_order("comments")
expected_order = move_to_end(move_to_front(@default_order, "t5"), "t8", "t7")
actual_order.should == expected_order
actual_order = thread_result_order("date", "desc")
actual_order = thread_result_order("date")
expected_order = move_to_front(@default_order, "t8", "t7")
actual_order.should == expected_order
actual_order = thread_result_order("date", "asc")
expected_order = move_to_front(@default_order.reverse, "t7", "t8")
actual_order.should == expected_order
end
context "pagination" do
def thread_result_page (sort_key, sort_order, page, per_page, course_id=DFLT_COURSE_ID, user_id=nil, unread=false)
get "/api/v1/threads", course_id: course_id, sort_key: sort_key, sort_order: sort_order, page: page, per_page: per_page, user_id: user_id, unread: unread
def thread_result_page (sort_key, page, per_page, course_id=DFLT_COURSE_ID, user_id=nil, unread=false)
get "/api/v1/threads", course_id: course_id, sort_key: sort_key, page: page, per_page: per_page, user_id: user_id, unread: unread
last_response.should be_ok
parse(last_response.body)
end
it "returns single page with no threads in a course" do
result = thread_result_page("date", "desc", 1, 20, "99")
result = thread_result_page("date", 1, 20, "99")
result["collection"].length.should == 0
result["thread_count"].should == 0
result["num_pages"].should == 1
result["page"].should == 1
end
it "returns single page" do
result = thread_result_page("date", "desc", 1, 20)
result = thread_result_page("date", 1, 20)
result["collection"].length.should == 10
result["thread_count"].should == 10
result["num_pages"].should == 1
result["page"].should == 1
end
it "returns multiple pages" do
result = thread_result_page("date", "desc", 1, 5)
result = thread_result_page("date", 1, 5)
result["collection"].length.should == 5
result["thread_count"].should == 10
result["num_pages"].should == 2
result["page"].should == 1
result = thread_result_page("date", "desc", 2, 5)
result = thread_result_page("date", 2, 5)
result["collection"].length.should == 5
result["thread_count"].should == 10
result["num_pages"].should == 2
......@@ -404,7 +350,7 @@ describe "app" do
end
it "returns page exceeding available pages with no results" do
#TODO: Review whether we can switch pagination endpoint to raise an exception; rather than an empty page
result = thread_result_page("date", "desc", 3, 5)
result = thread_result_page("date", 3, 5)
result["collection"].length.should == 0
result["thread_count"].should == 10
result["num_pages"].should == 2
......@@ -412,7 +358,7 @@ describe "app" do
end
def test_paged_order (sort_spec, expected_order, filter_spec=[], user_id=nil)
# sort spec is a hash with keys: sort_key, sort_dir, per_page
# sort spec is a hash with keys: sort_key, per_page
# filter spec is an array of filters to set, e.g. "unread", "flagged"
# expected order is an array of the expected titles of returned threads, in the expected order
actual_order = []
......@@ -422,7 +368,6 @@ describe "app" do
page = i + 1
result = thread_result_page(
sort_spec['sort_key'],
sort_spec['sort_dir'],
page,
per_page,
DFLT_COURSE_ID,
......@@ -446,20 +391,20 @@ describe "app" do
make_comment(@threads["t5"].author, @threads["t5"], "extra comment")
@threads["t7"].pinned = true
@threads["t7"].save!
expected_order = move_to_front(move_to_end(@default_order, "t5"), "t7")
test_paged_order({'sort_key' => 'comments', 'sort_dir' => 'asc', 'per_page' => 3}, expected_order)
expected_order = move_to_end(move_to_front(@default_order, "t5"), "t7")
test_paged_order({'sort_key' => 'comments', 'per_page' => 3}, expected_order)
end
it "orders correctly acrosss pages with unread filter" do
it "orders correctly across pages with unread filter" do
user = create_test_user(Random.new)
user.mark_as_read(@threads["t0"])
user.mark_as_read(@threads["t9"])
make_comment(@threads["t5"].author, @threads["t5"], "extra comment")
@threads["t7"].pinned = true
@threads["t7"].save!
expected_order = move_to_front(move_to_end(@default_order[1..8], "t5"), "t7")
expected_order = move_to_end(move_to_front(@default_order[1..8], "t5"), "t7")
test_paged_order(
{'sort_key' => 'comments', 'sort_dir' => 'asc', 'per_page' => 3},
{'sort_key' => 'comments', 'per_page' => 3},
expected_order,
["unread"],
user.id
......
spec/api/search_spec.rb
......@@ -21,18 +21,13 @@ describe "app" do
result.should == {}
end
it "returns an empty reuslt if text parameter is missing" do
it "returns an empty result if text parameter is missing" do
get "/api/v1/search/threads", course_id: course_id
assert_empty_response
end
it "returns an empty reuslt if sort key is invalid" do
get "/api/v1/search/threads", course_id: course_id, text: "foobar", sort_key: "invalid", sort_order: "desc"
assert_empty_response
end
it "returns an empty reuslt if sort order is invalid" do
get "/api/v1/search/threads", course_id: course_id, text: "foobar", sort_key: "date", sort_order: "invalid"
it "returns an empty result if sort key is invalid" do
get "/api/v1/search/threads", course_id: course_id, text: "foobar", sort_key: "invalid"
assert_empty_response
end
......@@ -159,8 +154,8 @@ describe "app" do
threads
end
def check_sort(sort_key, sort_order, expected_thread_indexes)
get "/api/v1/search/threads", text: "text", course_id: course_id, sort_key: sort_key, sort_order: sort_order
def check_sort(sort_key, expected_thread_indexes)
get "/api/v1/search/threads", text: "text", course_id: course_id, sort_key: sort_key
last_response.should be_ok
result = parse(last_response.body)
actual_ids = get_result_ids(result)
......@@ -170,28 +165,24 @@ describe "app" do
it "by date" do
asc_order = [0, 1, 2, 3, 4, 5]
check_sort("date", "asc", asc_order)
check_sort("date", "desc", asc_order.reverse)
check_sort("date", asc_order.reverse)
end
it "by activity" do
asc_order = [0, 1, 2, 3, 4, 5]
check_sort("activity", "asc", asc_order)
check_sort("activity", "desc", asc_order.reverse)
check_sort("activity", asc_order.reverse)
end
it "by votes" do
check_sort("votes", "asc", [5, 4, 3, 0, 2, 1])
check_sort("votes", "desc", [2, 1, 5, 4, 3, 0])
check_sort("votes", [2, 1, 5, 4, 3, 0])
end
it "by comments" do
check_sort("comments", "asc", [5, 4, 2, 0, 3, 1])
check_sort("comments", "desc", [3, 1, 5, 4, 2, 0])
check_sort("comments", [3, 1, 5, 4, 2, 0])
end
it "by default" do
check_sort(nil, nil, [5, 4, 3, 2, 1, 0])
check_sort(nil, [5, 4, 3, 2, 1, 0])
end
end
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment