From fcd7816d00bc73f37471a6cf3c91dfa14a2f858c Mon Sep 17 00:00:00 2001 From: Dan Minkevitch Date: Fri, 23 Jun 2017 15:13:40 -0700 Subject: [PATCH] Sort PlannerItems API by applicable dates Closes FALCOR-391 Test Plan: * As a student * In a course with Student Planner enabled * GET /api/v1/planner/items * The response should be returned in order by the objects' due dates, todo dates, or posted dates * GET the next page (bookmark) of results * The response should continue in the same order Change-Id: Ic0b6ed8f51551ff8839cc11913e94b26709bc0c7 Reviewed-on: https://gerrit.instructure.com/116478 Tested-by: Jenkins Reviewed-by: Steven Burnett QA-Review: Dan Sasaki Product-Review: Dan Minkevitch --- .../planner_overrides_controller.rb | 12 ++-- app/models/discussion_topic.rb | 6 +- .../planner_overrides_controller_spec.rb | 61 +++++++++++++++++++ 3 files changed, 69 insertions(+), 10 deletions(-) diff --git a/app/controllers/planner_overrides_controller.rb b/app/controllers/planner_overrides_controller.rb index f78c0ddabe6..6c495976079 100644 --- a/app/controllers/planner_overrides_controller.rb +++ b/app/controllers/planner_overrides_controller.rb @@ -295,7 +295,7 @@ class PlannerOverridesController < ApplicationController each_with_object([]) do |(scope_name, scope), all_scopes| next if scope.blank? base_model = scope_name == :ungraded_quiz ? Quizzes::Quiz : Assignment - collection = item_collection(scope_name.to_s, scope, base_model, :id) + collection = item_collection(scope_name.to_s, scope, base_model, :due_at, :created_at, :id) all_scopes << collection end scopes @@ -305,7 +305,7 @@ class PlannerOverridesController < ApplicationController item_collection('unread_discussion_topics', DiscussionTopic.active.todo_date_between(start_date, end_date). unread_for(@current_user), - DiscussionTopic, :id) + DiscussionTopic, :todo_date, :posted_at, :delayed_post_at, :last_reply_at, :created_at, :id) end def unread_submission_collection @@ -313,25 +313,25 @@ class PlannerOverridesController < ApplicationController Assignment.active.joins(:submissions). where(submissions: {id: Submission.unread_for(@current_user).pluck(:id)}). due_between_with_overrides(start_date, end_date), - Assignment, :id) + Assignment, :due_at, :created_at, :id) end def planner_note_collection item_collection('planner_notes', PlannerNote.where(user: @current_user, todo_date: @start_date...@end_date), - PlannerNote, :id) + PlannerNote, :todo_date, :created_at, :id) end def page_collection item_collection('pages', @current_user.wiki_pages_needing_viewing(default_opts), - WikiPage, :id) + WikiPage, :todo_date, :created_at, :id) end def ungraded_discussion_collection item_collection('ungraded_discussions', @current_user.discussion_topics_needing_viewing(default_opts), - DiscussionTopic, :id) + DiscussionTopic, :todo_date, :posted_at, :delayed_post_at, :last_reply_at, :created_at, :id) end def item_collection(label, scope, base_model, *order_by) diff --git a/app/models/discussion_topic.rb b/app/models/discussion_topic.rb index bb4a4fd9be8..3cb90338465 100644 --- a/app/models/discussion_topic.rb +++ b/app/models/discussion_topic.rb @@ -521,8 +521,7 @@ class DiscussionTopic < ActiveRecord::Base where("discussion_topic_participants.id IS NOT NULL AND (discussion_topic_participants.user_id = :user AND discussion_topic_participants.workflow_state = 'read')", - user: user). - distinct + user: user) } scope :unread_for, lambda { |user| # TODO: Fix for when participants doesn't include user @@ -530,8 +529,7 @@ class DiscussionTopic < ActiveRecord::Base where("discussion_topic_participants.id IS NULL OR (discussion_topic_participants.user_id = :user AND discussion_topic_participants.workflow_state <> 'read')", - user: user). - distinct + user: user) } scope :published, -> { where("discussion_topics.workflow_state = 'active'") } diff --git a/spec/controllers/planner_overrides_controller_spec.rb b/spec/controllers/planner_overrides_controller_spec.rb index 5909f9ba7c3..af02e83fb67 100644 --- a/spec/controllers/planner_overrides_controller_spec.rb +++ b/spec/controllers/planner_overrides_controller_spec.rb @@ -174,6 +174,67 @@ describe PlannerOverridesController do end end + context "date sorting" do + it "should return results in order by object type then date" do + wiki_page_model(course: @course) + @page.todo_date = 1.day.from_now + @page.save! + @assignment3 = course_assignment + @assignment3.due_at = 1.week.ago + @assignment3.save! + get :items_index + response_json = json_parse(response.body) + expect(response_json.length).to eq 4 + expect(response_json.map { |i| i["plannable_id"] }).to eq [@assignment3.id, @assignment.id, @assignment2.id, @page.id] + end + end + + context "pagination" do + PER_PAGE = 5 + + def test_page(idx = 0, bookmark = nil) + opts = { per_page: PER_PAGE } + opts.merge(page: bookmark) if bookmark.present? + + page = get :items_index, opts + links = Api.parse_pagination_links(page.headers['Link']) + response_json = json_parse(page.body) + expect(response_json.length).to eq PER_PAGE + ids = response_json.map { |i| i["plannable_id"] } + expected_ids = [] + PER_PAGE.times.with_index(idx) {|i| expected_ids << @assignments[i].id} + expect(ids).to eq expected_ids + + links.detect { |l| l[:rel] == "next" }["page"] + end + + before :once do + @assignments = [] + 20.downto(0) do |i| + asg = course_assignment + asg.due_at = i.days.ago + asg.save! + @assignments << asg + end + end + + it "should adhere to per_page" do + get :items_index, per_page: 2 + response_json = json_parse(response.body) + expect(response_json.length).to eq 2 + expect(response_json.map { |i| i["plannable_id"] }).to eq [@assignments[0].id, @assignments[1].id] + end + + it "should paginate results in correct order" do + next_page = '' + 10.times do |i| + next_page = test_page(i, next_page) + end + end + + end + + context "new activity filter" do it "should return newly created & unseen items" do dt = @course.discussion_topics.create!(title: "Yes", message: "Please", user: @teacher, todo_date: Time.zone.now)