add "mark all as read" command to discussion topic gear menu

fixes CNVS-5714 test plan: - create a discussion topic as one user. - as another user, make a bunch of replies (more than a pages worth). - as the first user, refresh the topic and see a bunch of unread replies (some will get auto marked). - manually mark some of the read posts as unread. - choose the new "mark all as read option" from the gear menu. - blue ball should turn gray on all posts. - go to discussion index, unread counter on the topic should show no unread posts - go back to discussion, unread counter should show no unread posts. Change-Id: I5ffe8717731943349996e6f098d491aca5bc43a1 Reviewed-on: https://gerrit.instructure.com/20617 Tested-by: Jenkins <jenkins@instructure.com> Reviewed-by: Mark Ericksen <marke@instructure.com> QA-Review: Cam Theriault <cam@instructure.com> Product-Review: Jon Willesen <jonw@instructure.com>
2013-05-13 16:10:11 -06:00 · 2013-05-13 16:10:11 -06:00 · 690dd65081
parent 648c3c3aab
commit 690dd65081
10 changed files with 153 additions and 68 deletions
--- a/app/coffeescripts/bundles/discussion.coffee
+++ b/app/coffeescripts/bundles/discussion.coffee
@ -74,6 +74,13 @@ require [
    e = data.flattened[id]
    e.read_state = read_state if e

+  ##
+  # propagate mark all read/unread changes to all views
+  setAllReadStateAllViews = (newReadState) ->
+    entries.setAllReadState(newReadState)
+    EntryView.setAllReadState(newReadState)
+    filterView.setAllReadState(newReadState)
+
  entriesView.on 'scrollAwayFromEntry', ->
    # prevent scroll to top for non-pushstate browsers when hash changes
    top = $container.scrollTop()
@ -112,6 +119,14 @@ require [
    EntryView.collapseRootEntries()
    scrollToTop()

+  toolbarView.on 'markAllAsRead', ->
+    data.markAllAsRead()
+    setAllReadStateAllViews('read')
+
+  toolbarView.on 'markAllAsUnread', ->
+    data.markAllAsUnread()
+    setAllReadStateAllViews('unread')
+
  filterView.on 'render', ->
    scrollToTop()

--- a/app/coffeescripts/collections/EntryCollection.coffee
+++ b/app/coffeescripts/collections/EntryCollection.coffee
@ -33,6 +33,10 @@ define [
      page.fullCollection = this
      page

+    setAllReadState: (newReadState) ->
+      @each (entry) ->
+        entry.set 'read_state', newReadState
+
    ##
    # This could have been two or three well-named methods, but it doesn't make
    # a whole lot of sense to walk the tree over and over to get each piece of
--- a/app/coffeescripts/models/Topic.coffee
+++ b/app/coffeescripts/models/Topic.coffee
@ -4,12 +4,14 @@

 define [
  'i18n!discussions'
+  'jquery'
  'underscore'
  'Backbone'
  'compiled/util/BackoffPoller'
  'compiled/arr/walk'
  'compiled/arr/erase'
-], (I18n, {each}, Backbone, BackoffPoller, walk, erase) ->
+  'jquery.ajaxJSON'
+], (I18n, $, {each}, Backbone, BackoffPoller, walk, erase) ->

  UNKNOWN_AUTHOR =
    avatar_image_url: null
@ -45,6 +47,18 @@ define [
        backoffFactor: 1.6
      loader.start()

+    markAllAsRead: ->
+      $.ajaxJSON ENV.DISCUSSION.MARK_ALL_READ_URL, 'PUT', forced_read_state: false
+      @setAllReadState('read')
+
+    markAllAsUnread: ->
+      $.ajaxJSON ENV.DISCUSSION.MARK_ALL_UNREAD_URL, 'DELETE', forced_read_state: false
+      @setAllReadState('unread')
+
+    setAllReadState: (newReadState) ->
+      each @flattened, (entry) ->
+        entry.read_state = newReadState
+
    parse: (data, status, xhr) ->
      @data = data
      # build up entries in @data.entries, mainly because we don't want deleted
--- a/app/coffeescripts/views/DiscussionTopic/DiscussionFilterResultsView.coffee
+++ b/app/coffeescripts/views/DiscussionTopic/DiscussionFilterResultsView.coffee
@ -22,6 +22,11 @@ define [
    attach: ->
      @model.on 'change', @renderOrTeardownResults

+    setAllReadState: (newReadState) ->
+      if @collection?
+        @collection.fullCollection.each (entry) ->
+          entry.set 'read_state', newReadState
+
    resetCollection: (models) =>
      collection = new EntryCollection models, perPage: 10
      @collection = collection.getPageAsCollection 0
--- a/app/coffeescripts/views/DiscussionTopic/DiscussionToolbarView.coffee
+++ b/app/coffeescripts/views/DiscussionTopic/DiscussionToolbarView.coffee
@ -19,6 +19,8 @@ define [
      'change #onlyUnread': 'toggleUnread'
      'click #collapseAll': 'collapseAll'
      'click #expandAll': 'expandAll'
+      'click .mark_all_as_read': 'markAllAsRead'
+      'click .mark_all_as_unread': 'markAllAsUnread'

    initialize: ->
      @model.on 'change', @clearInputs
@ -59,6 +61,14 @@ define [
      @model.set 'collapsed', false
      @trigger 'expandAll'

+    markAllAsRead: (event) ->
+      event.preventDefault()
+      @trigger 'markAllAsRead'
+
+    markAllAsUnread: (event) ->
+      event.preventDefault()
+      @trigger 'markAllAsUnread'
+
    maybeDisableFields: ->
      @$disableWhileFiltering.attr 'disabled', @model.hasFilter()

--- a/app/coffeescripts/views/DiscussionTopic/EntryView.coffee
+++ b/app/coffeescripts/views/DiscussionTopic/EntryView.coffee
@ -30,6 +30,10 @@ define [
      _.each @instances, (view) ->
        view.expand() unless view.model.get 'parent'

+    @setAllReadState = (newReadState) ->
+      _.each @instances, (view) ->
+        view.model.set 'read_state', newReadState
+
    els:
      '.discussion_entry:first': '$entryContent'
      '.replies:first': '$replies'
--- a/app/coffeescripts/views/DiscussionTopic/FilterEntryView.coffee
+++ b/app/coffeescripts/views/DiscussionTopic/FilterEntryView.coffee
@ -23,7 +23,7 @@ define [

    initialize: ->
      super
-      @model.on 'change:read_state', @toggleReadState
+      @model.on 'change:read_state', @updateReadState

    toJSON: ->
      @model.attributes
@ -33,7 +33,7 @@ define [

    afterRender: ->
      super
-      @setToggleTooltip()
+      @updateReadState()

    toggleRead: (e) ->
      e.stopPropagation()
@ -43,12 +43,12 @@ define [
      else
        @model.markAsRead()

-    toggleReadState: (model, read_state) =>
-      @setToggleTooltip()
-      @$entryContent.toggleClass 'unread', read_state is 'unread'
-      @$entryContent.toggleClass 'read', read_state is 'read'
+    updateReadState: =>
+      @updateTooltip()
+      @$entryContent.toggleClass 'unread', @model.get('read_state') is 'unread'
+      @$entryContent.toggleClass 'read', @model.get('read_state') is 'read'

-    setToggleTooltip: ->
+    updateTooltip: ->
      tooltip = if @model.get('read_state') is 'unread'
        I18n.t('mark_as_read', 'Mark as Read')
      else
--- a/app/controllers/discussion_topics_controller.rb
+++ b/app/controllers/discussion_topics_controller.rb
@ -260,6 +260,8 @@ class DiscussionTopicsController < ApplicationController
              :UPDATE_URL => named_context_url(@context, :api_v1_context_discussion_update_reply_url, @topic, ':id'),
              :MARK_READ_URL => named_context_url(@context, :api_v1_context_discussion_topic_discussion_entry_mark_read_url, @topic, ':id'),
              :MARK_UNREAD_URL => named_context_url(@context, :api_v1_context_discussion_topic_discussion_entry_mark_unread_url, @topic, ':id'),
+              :MARK_ALL_READ_URL => named_context_url(@context, :api_v1_context_discussion_topic_mark_all_read_url, @topic),
+              :MARK_ALL_UNREAD_URL => named_context_url(@context, :api_v1_context_discussion_topic_mark_all_unread_url, @topic),
              :MANUAL_MARK_AS_READ => @current_user.manual_mark_as_read?,
              :CURRENT_USER => user_display_json(@current_user),
              :INITIAL_POST_REQUIRED => @initial_post_required,
--- a/app/views/discussion_topics/show.html.erb
+++ b/app/views/discussion_topics/show.html.erb
@ -72,65 +72,65 @@
          </a>
        <% end %>

-        <% if can_do(@topic, @current_user, :delete) ||
-              @presenter.can_grade?(@current_user) ||
-              @presenter.show_peer_reviews?(@current_user) ||
-              @presenter.should_show_rubric?(@current_user) %>
-          <div class="admin-links">
-            <a class="al-trigger btn" data-kyle-menu-options='{"appendMenuTo": "body"}'>
-              <i class="icon-settings"></i><i class="icon-mini-arrow-down"></i>
-              <div class="screenreader-only"><%= t :manage_discussion, 'Manage Discussion' %></div>
-            </a>
-            <ul class="al-options">
-              <% if can_do(@topic, @current_user, :delete) %>
-                <li><a href="<%= context_url(@context, :context_discussion_topic_url, @topic.id) %>" data-method="delete" rel="nofollow" data-confirm="<%= t :confirm_delete_discussion, 'Are you sure you want to delete this discussion?' %>"><i class="icon-trash"></i> <%= t :delete, 'Delete' %></a></li>
-              <% end %>
+        <div class="admin-links">
+          <a class="al-trigger btn" data-kyle-menu-options='{"appendMenuTo": "body"}'>
+            <i class="icon-settings"></i><i class="icon-mini-arrow-down"></i>
+            <div class="screenreader-only"><%= t :manage_discussion, 'Manage Discussion' %></div>
+          </a>
+          <ul class="al-options">
+            <li><a href="#" class="mark_all_as_read"><%#= <i class="icon-read" aria-hidden="hidden"></i> %> <%= t :mark_all_as_read, 'Mark All as Read' %></a></li>
+            <% if false # keep this command out of the interface for now %>
+              <li><a href="#" class="mark_all_as_unread"><%#= <i class="icon-message" aria-hidden="hidden"></i> %> <%= t :mark_all_as_unread, 'Mark All as Unread' %></a></li>
+            <% end %>

-              <% if @presenter.can_grade?(@current_user) && @presenter.allows_speed_grader? %>
-                <li><a href="<%= context_url(@topic.assignment.context,
-                                             :speed_grader_context_gradebook_url,
-                                             :assignment_id => @topic.assignment.id) %>">
-                  <i class="icon-speed-grader" aria-hidden="hidden"></i>
-                  <%= t :speed_grader, "Speed Grader" %>
-                </a></li>
-              <% end %>
+            <% if can_do(@topic, @current_user, :delete) %>
+              <li><a href="<%= context_url(@context, :context_discussion_topic_url, @topic.id) %>" class="delete_discussion" data-method="delete" rel="nofollow" data-confirm="<%= t :confirm_delete_discussion, 'Are you sure you want to delete this discussion?' %>"><i class="icon-trash"></i> <%= t :delete, 'Delete' %></a></li>
+            <% end %>

-              <% if @presenter.show_peer_reviews?(@current_user) %>
-                <li><a class="peer-review assignment_peer_reviews_link"
-                   href="<%= context_url(@topic.assignment.context,
-                                         :context_assignment_peer_reviews_url,
-                                         @topic.assignment.id) %>">
-                  <i class="icon-peer-review" aria-hidden="hidden"></i>
-                  <%= t 'links.peer_reviews', "Peer Reviews" %>
-                </a></li>
-              <% end %>
+            <% if @presenter.can_grade?(@current_user) && @presenter.allows_speed_grader? %>
+              <li><a href="<%= context_url(@topic.assignment.context,
+                                           :speed_grader_context_gradebook_url,
+                                           :assignment_id => @topic.assignment.id) %>">
+                <i class="icon-speed-grader" aria-hidden="hidden"></i>
+                <%= t :speed_grader, "Speed Grader" %>
+              </a></li>
+            <% end %>

-              <% if @presenter.should_show_rubric?(@current_user) %>
-                <li>
-                  <%# HACK! this is here because edit_rubric.js expects there to be a #add_rubric_url on the page and sets it's <form action="..."> to it %>
-                  <% if can_do(@topic.assignment, @current_user, :update) %>
-                    <a href="<%= context_url(@topic.assignment.context, :context_rubrics_url) %>" id="add_rubric_url" style="display: none;"></a>
-                  <% end %>
+            <% if @presenter.show_peer_reviews?(@current_user) %>
+              <li><a class="peer-review assignment_peer_reviews_link"
+                 href="<%= context_url(@topic.assignment.context,
+                                       :context_assignment_peer_reviews_url,
+                                       @topic.assignment.id) %>">
+                <i class="icon-peer-review" aria-hidden="hidden"></i>
+                <%= t 'links.peer_reviews', "Peer Reviews" %>
+              </a></li>
+            <% end %>

-                  <a class="rubric_dialog_trigger rubric" href="#" data-no-rubric-exists="<%= !@presenter.has_attached_rubric? %>" data-url="<%= context_url(@topic.assignment.context, :context_assignment_rubric_url, @topic.assignment.id) %>">
-                    <i class="icon-rubric" aria-hidden="hidden"></i>
-                    <%= @presenter.has_attached_rubric? ? t(:show_rubric, "Show Rubric") : t(:add_rubric, "Add Rubric") %>
-                  </a>
-                </li>
-              <% end %>
-
-              <% unless @presenter.comments_disabled? %>
-                <% if @locked %>
-                  <% if can_do(@context, @current_user, :moderate_forum) %>
-                    <li><a href="#" data-mark-locked="false" class="discussion_locked_toggler"><i class="icon-unlock"></i> <%= t(:unlock_topic, %{Open for Comments}) %></a></li>
-                  <% end %>
-                <% elsif can_do(@context, @current_user, :moderate_forum) && (!@topic.assignment.try(:due_at) || @topic.assignment.due_at <= Time.now) %>
-                  <li><a href="#" data-mark-locked="true" class="discussion_locked_toggler"><i class="icon-lock"></i> <%= t(:lock_topic, %{Close for Comments}) %></a></li>
+            <% if @presenter.should_show_rubric?(@current_user) %>
+              <li>
+                <%# HACK! this is here because edit_rubric.js expects there to be a #add_rubric_url on the page and sets it's <form action="..."> to it %>
+                <% if can_do(@topic.assignment, @current_user, :update) %>
+                  <a href="<%= context_url(@topic.assignment.context, :context_rubrics_url) %>" id="add_rubric_url" style="display: none;"></a>
                <% end %>
+
+                <a class="rubric_dialog_trigger rubric" href="#" data-no-rubric-exists="<%= !@presenter.has_attached_rubric? %>" data-url="<%= context_url(@topic.assignment.context, :context_assignment_rubric_url, @topic.assignment.id) %>">
+                  <i class="icon-rubric" aria-hidden="hidden"></i>
+                  <%= @presenter.has_attached_rubric? ? t(:show_rubric, "Show Rubric") : t(:add_rubric, "Add Rubric") %>
+                </a>
+              </li>
+            <% end %>
+
+            <% unless @presenter.comments_disabled? %>
+              <% if @locked %>
+                <% if can_do(@context, @current_user, :moderate_forum) %>
+                  <li><a href="#" data-mark-locked="false" class="discussion_locked_toggler"><i class="icon-unlock"></i> <%= t(:unlock_topic, %{Open for Comments}) %></a></li>
+                <% end %>
+              <% elsif can_do(@context, @current_user, :moderate_forum) && (!@topic.assignment.try(:due_at) || @topic.assignment.due_at <= Time.now) %>
+                <li><a href="#" data-mark-locked="true" class="discussion_locked_toggler"><i class="icon-lock"></i> <%= t(:lock_topic, %{Close for Comments}) %></a></li>
              <% end %>
-            </ul>
-          </div>
-        <% end %>
+            <% end %>
+          </ul>
+        </div>
      </div>
    </div>
  </div>
--- a/spec/selenium/discussions_spec.rb
+++ b/spec/selenium/discussions_spec.rb
@ -168,7 +168,7 @@ describe "discussions" do
      it "should validate closing the discussion for comments" do
        create_and_go_to_topic
        f("#discussion-toolbar .al-trigger").click
-        expect_new_page_load { f("#ui-id-3").click }
+        expect_new_page_load { f(".discussion_locked_toggler").click }
        f('.discussion-fyi').text.should == 'This topic is closed for comments'
        ff('.discussion-reply-label').should be_empty
        DiscussionTopic.last.workflow_state.should == 'locked'
@ -177,7 +177,7 @@ describe "discussions" do
      it "should validate reopening the discussion for comments" do
        create_and_go_to_topic('closed discussion', 'side_comment', true)
        f("#discussion-toolbar .al-trigger").click
-        expect_new_page_load { f("#ui-id-3").click }
+        expect_new_page_load { f(".discussion_locked_toggler").click }
        ff('.discussion-reply-label').should_not be_empty
        DiscussionTopic.last.workflow_state.should == 'active'
      end
@ -603,7 +603,7 @@ describe "discussions" do
          wait_for_ajaximations

          f("#discussion-toolbar .al-trigger").click
-          expect_new_page_load { f("#ui-id-3").click }
+          expect_new_page_load { f(".discussion_locked_toggler").click }

          @topic.reload
          @topic.delayed_post_at.should be_nil
@ -751,12 +751,16 @@ describe "discussions" do
      f('#new-discussion-btn').should be_nil
    end

-    it "should not show an empty gear menu to students who've created a discussion" do
+    it "should not show admin options in gear menu to students who've created a discussion" do
      @student_topic = @course.discussion_topics.create!(:user => @student, :message => 'student topic', :discussion_type => 'side_comment')
      @student_entry = @student_topic.discussion_entries.create!(:user => @student, :message => 'student entry')
      get "/courses/#{@course.id}/discussion_topics/#{@student_topic.id}"
      wait_for_ajax_requests
-      f('.headerBar .admin-links').should be_nil
+      f('.headerBar .admin-links').should_not be_nil
+      f('.mark_all_as_read').should_not be_nil
+      #f('.mark_all_as_unread').should_not be_nil
+      f('.delete_discussion').should be_nil
+      f('.discussion_locked_toggler').should be_nil
    end

    it "should allow students to reply to a discussion even if they cannot create a topic" do
@ -970,11 +974,14 @@ describe "discussions" do
  end

  context "marking as read" do
-    it "should mark things as read" do
-      reply_count = 2
+    before do
      course_with_student
      course_with_teacher_logged_in(:course => @course)
      @topic = @course.discussion_topics.create!(:title => 'mark as read test', :message => 'test mark as read', :user => @student)
+    end
+
+    it "should automatically mark things as read" do
+      reply_count = 2
      reply_count.times { @topic.discussion_entries.create!(:message => 'Lorem ipsum dolor sit amet', :user => @student) }
      @topic.create_materialized_view

@ -1006,5 +1013,29 @@ describe "discussions" do
      wait_for_ajaximations
      ff(".discussion_entry.unread").size.should == 1
    end
+
+    it "should mark all as read" do
+      reply_count = 8
+      (reply_count / 2).times do |n|
+        entry = @topic.reply_from(:user => @student, :text => "entry #{n}")
+        entry.reply_from(:user => @student, :text => "sub reply #{n}")
+      end
+      @topic.create_materialized_view
+
+      # so auto mark as read won't mess up this test
+      @teacher.preferences[:manual_mark_as_read] = true
+      @teacher.save!
+
+      go_to_topic
+
+      ff('.discussion-entries .unread').length.should == reply_count
+      ff('.discussion-entries .read').length.should == 0
+
+      f("#discussion-toolbar .al-trigger").click
+      f('.mark_all_as_read').click
+      wait_for_ajaximations
+      ff('.discussion-entries .unread').length.should == 0
+      ff('.discussion-entries .read').length.should == reply_count
+    end
  end
 end