From cd0ea052eda3bf3bba7ec3f6ef3ba14c7e2980d5 Mon Sep 17 00:00:00 2001
From: Greg Price <gprice@edx.org>
Date: Thu, 4 Sep 2014 17:56:49 -0400
Subject: [PATCH] Ensure forum nav cohort selector works correctly

One minor case is fixed (to pass "" instead of "all" as the group_id
parameter to indicate viewing all threads), and tests are added.
---
 .../discussion_thread_list_view_spec.coffee   | 63 ++++++++++++++++---
 .../discussion/_thread_list_template.html     |  2 +-
 2 files changed, 54 insertions(+), 11 deletions(-)

diff --git a/common/static/coffee/spec/discussion/view/discussion_thread_list_view_spec.coffee b/common/static/coffee/spec/discussion/view/discussion_thread_list_view_spec.coffee
index 1b2940e54d..11866c349c 100644
--- a/common/static/coffee/spec/discussion/view/discussion_thread_list_view_spec.coffee
+++ b/common/static/coffee/spec/discussion/view/discussion_thread_list_view_spec.coffee
@@ -59,7 +59,7 @@ describe "DiscussionThreadListView", ->
                     <li
                         class="forum-nav-browse-menu-item"
                         data-discussion-id="other"
-                        data-cohorted="false"
+                        data-cohorted="true"
                     >
                         <a href="#" class="forum-nav-browse-title">Other Category</a>
                     </li>
@@ -75,6 +75,14 @@ describe "DiscussionThreadListView", ->
                             <option value="flagged">Flagged</option>
                         </select>
                     </label>
+                    <label class="forum-nav-filter-cohort">
+                        <span class="sr">Cohort:</span>
+                        <select class="forum-nav-filter-cohort-control">
+                            <option value="">in all cohorts</option>
+                            <option value="1">Cohort1</option>
+                            <option value="2">Cohort2</option>
+                        </select>
+                    </label>
                     <label class="forum-nav-sort">
                         <select class="forum-nav-sort-control">
                             <option value="date">by recent activity</option>
@@ -118,6 +126,15 @@ describe "DiscussionThreadListView", ->
         @view = new DiscussionThreadListView({collection: @discussion, el: $("#fixture-element")})
         @view.render()
 
+    setupAjax = (callback) ->
+      $.ajax.andCallFake(
+        (params) =>
+          if callback
+            callback(params)
+          params.success({discussion_data: [], page: 1, num_pages: 1})
+          {always: ->}
+      )
+
     renderSingleThreadWithProps = (props) ->
       makeView(new Discussion([new Thread(DiscussionViewSpecHelper.makeThreadWithProps(props))])).render()
 
@@ -146,6 +163,23 @@ describe "DiscussionThreadListView", ->
                 expect($.ajax).toHaveBeenCalled()
         )
 
+    describe "cohort selector", ->
+        it "should filter correctly", ->
+            expectedGroupId = null
+            setupAjax((params) => expect(params.data.group_id).toEqual(expectedGroupId))
+            _.each(
+                [
+                    {val: "", expectedGroupId: undefined},
+                    {val: "1", expectedGroupId: "1"},
+                    {val: "2", expectedGroupId: "2"}
+                ],
+                (optionInfo) =>
+                    expectedGroupId = optionInfo.expectedGroupId
+                    @view.$(".forum-nav-filter-cohort-control").val(optionInfo.val).change()
+                    expect($.ajax).toHaveBeenCalled()
+                    $.ajax.reset()
+            )
+
     it "search should clear filter", ->
         expectFilter(null)
         @view.$(".forum-nav-filter-main-control").val("flagged")
@@ -388,15 +422,6 @@ describe "DiscussionThreadListView", ->
         expect($(".forum-nav-thread-labels").length).toEqual(0)
 
     describe "browse menu", ->
-      setupAjax = (callback) ->
-        $.ajax.andCallFake(
-          (params) =>
-            if callback
-              callback(params)
-            params.success({discussion_data: [], page: 1, num_pages: 1})
-            {always: ->}
-        )
-
       afterEach ->
         # Remove handler added to make browse menu disappear
         $("body").unbind("click")
@@ -472,6 +497,24 @@ describe "DiscussionThreadListView", ->
           $(".forum-nav-browse-menu-following .forum-nav-browse-title").click()
           expect($(".forum-nav-browse-current").text()).toEqual("Posts I'm Following")
 
+        it "should show/hide the cohort selector", ->
+          setupAjax()
+          _.each(
+            [
+              {selector: ".forum-nav-browse-menu-all", cohortVisibility: true},
+              {selector: ".forum-nav-browse-menu-following", cohortVisibility: false},
+              {
+                selector: ".forum-nav-browse-menu-item:has(.forum-nav-browse-menu-item .forum-nav-browse-menu-item)",
+                cohortVisibility: false
+              },
+              {selector: "[data-discussion-id=child]", cohortVisibility: false},
+              {selector: "[data-discussion-id=other]", cohortVisibility: true}
+            ],
+            (itemInfo) =>
+              @view.$("#{itemInfo.selector} > .forum-nav-browse-title").click()
+              expect(@view.$(".forum-nav-filter-cohort").is(":visible")).toEqual(itemInfo.cohortVisibility)
+          )
+
         testSelectionRequest = (callback, itemText) ->
           setupAjax(callback)
           $(".forum-nav-browse-title:contains(#{itemText})").click()
diff --git a/lms/templates/discussion/_thread_list_template.html b/lms/templates/discussion/_thread_list_template.html
index 66ef69120f..48ef678d0c 100644
--- a/lms/templates/discussion/_thread_list_template.html
+++ b/lms/templates/discussion/_thread_list_template.html
@@ -44,7 +44,7 @@
                 ## Translators: This labels a cohort menu in forum navigation
                 <span class="sr">${_("Cohort:")}</span>
                 <select class="forum-nav-filter-cohort-control">
-                    <option value="all">${_("in all cohorts")}</option>
+                    <option value="">${_("in all cohorts")}</option>
                         %for c in cohorts:
                             <option value="${c['id']}">${c['name']}</option>
                         %endfor