fix faculty journal regression, closes #5724

incidentally also fixes issue where user's common contexts were not
displaying in the recipient finder

Change-Id: I69c866ead86796a3602030c8fdd1df25200fdcb3
Reviewed-on: https://gerrit.instructure.com/6033
Tested-by: Hudson <hudson@instructure.com>
Reviewed-by: Zach Pendleton <zachp@instructure.com>

app/coffeescripts/messages.coffee

@@ -614,12 +614,8 @@ I18n.scoped 'conversations', (I18n) ->
       for user in data.participants when !MessageInbox.user_cache[user.id]?.avatar_url
         MessageInbox.user_cache[user.id] = user
         user.html_name = html_name_for_user(user)
-      $form.find('#user_note_info').showIf(
+      if data['private'] and user = (user for user in data.participants when user.id isnt MessageInbox.user_id)[0] and can_add_notes_for(user)
-          $c.hasClass('private') and
+        $form.find('#user_note_info').show()
-          (user_id = $c.find('.participant').first().data('id')) and
-          (user = MessageInbox.user_cache[user_id]) and
-          can_add_notes_for(user)
-        )
       inbox_resize()
       $messages.show()
       i = j = 0
@@ -1499,7 +1495,8 @@ I18n.scoped 'conversations', (I18n) ->
         unless data.id and "#{data.id}".match(/^(course|group)_/)
           data = $.extend({}, data)
           delete data.avatar_url # since it's the wrong size and possibly a blank image
-          MessageInbox.user_cache[data.id] ?= data
+          current_data = MessageInbox.user_cache[data.id] ? {}
+          MessageInbox.user_cache[data.id] = $.extend(current_data, data)
       selector:
         messages: {no_results: I18n.t('no_results', 'No results found')}
         populator: ($node, data, options={}) ->
@@ -1514,7 +1511,7 @@ I18n.scoped 'conversations', (I18n) ->
           $b.text(data.name)
           $span = $('<span />')
           if data.common_courses?
-            $span.text(MessageInbox.context_list(data))
+            $span.text(MessageInbox.context_list(courses: data.common_courses, groups: data.common_groups))
           else if data.type and data.user_count?
             $span.text(I18n.t('people_count', 'person', {count: data.user_count}))
           $node.append($b, $context_name, $span)

public/javascripts/compiled/messages.js

@@ -836,7 +836,7 @@
       $form.loadingImage();
       $c = $selected_conversation;
       completion = function(data) {
-        var i, j, message, submission, user, user_id, _i, _len, _ref, _ref2;
+        var i, j, message, submission, user, _i, _len, _ref, _ref2;
         if (!is_selected($c)) {
           return;
         }
@@ -848,7 +848,20 @@
             user.html_name = html_name_for_user(user);
           }
         }
-        $form.find('#user_note_info').showIf($c.hasClass('private') && (user_id = $c.find('.participant').first().data('id')) && (user = MessageInbox.user_cache[user_id]) && can_add_notes_for(user));
+        if (data['private'] && (user = ((function() {
+          var _j, _len2, _ref3, _results;
+          _ref3 = data.participants;
+          _results = [];
+          for (_j = 0, _len2 = _ref3.length; _j < _len2; _j++) {
+            user = _ref3[_j];
+            if (user.id !== MessageInbox.user_id) {
+              _results.push(user);
+            }
+          }
+          return _results;
+        })())[0] && can_add_notes_for(user))) {
+          $form.find('#user_note_info').show();
+        }
         inbox_resize();
         $messages.show();
         i = j = 0;
@@ -1994,7 +2007,7 @@
       $('.recipients').tokenInput({
         placeholder: I18n.t('recipient_field_placeholder', "Enter a name, course, or group"),
         added: function(data, $token, new_token) {
-          var $details, _base, _name, _ref3;
+          var $details, current_data, _ref3;
           if (new_token && data.type) {
             $token.addClass(data.type);
             if (data.user_count != null) {
@@ -2009,7 +2022,8 @@
           if (!(data.id && ("" + data.id).match(/^(course|group)_/))) {
             data = $.extend({}, data);
             delete data.avatar_url;
-            return (_ref3 = (_base = MessageInbox.user_cache)[_name = data.id]) != null ? _ref3 : _base[_name] = data;
+            current_data = (_ref3 = MessageInbox.user_cache[data.id]) != null ? _ref3 : {};
+            return MessageInbox.user_cache[data.id] = $.extend(current_data, data);
           }
         },
         selector: {
@@ -2035,7 +2049,10 @@
             $b.text(data.name);
             $span = $('<span />');
             if (data.common_courses != null) {
-              $span.text(MessageInbox.context_list(data));
+              $span.text(MessageInbox.context_list({
+                courses: data.common_courses,
+                groups: data.common_groups
+              }));
             } else if (data.type && (data.user_count != null)) {
               $span.text(I18n.t('people_count', 'person', {
                 count: data.user_count

spec/selenium/conversations_spec.rb

@@ -17,8 +17,9 @@ shared_examples_for "conversations selenium tests" do
   def submit_message_form(opts={})
     opts[:message] ||= "Test Message"
     opts[:attachments] ||= []
+    opts[:add_recipient] = true unless opts.has_key?(:add_recipient)
 
-    if browser = find_with_jquery("#create_message_form .browser:visible")
+    if opts[:add_recipient] && browser = find_with_jquery("#create_message_form .browser:visible")
       browser.click
       keep_trying_until{
         if elem = find_with_jquery('.selectable:visible')
@@ -171,6 +172,69 @@ shared_examples_for "conversations selenium tests" do
       find_with_jquery("#{message} .message_attachments li:last a .title").text.should == file2[0]
     end
   end
+
+  context "user notes" do
+    before do
+      @the_teacher = User.create(:name => "teacher bob")
+      @course.enroll_teacher(@the_teacher)
+      @the_student = User.create(:name => "student bob")
+      @course.enroll_student(@the_student)
+    end
+
+    def add_recipient(search)
+      input = find_with_jquery("#create_message_form input:visible")
+      input.send_keys(search)
+      keep_trying_until{ driver.execute_script("return $('#recipients').data('token_input').selector.last_search") == search }
+      input.send_keys(:return)
+    end
+
+    it "should not allow user notes if not enabled" do
+      @course.account.update_attribute :enable_user_notes, false
+      new_conversation
+      add_recipient("student bob")
+      driver.find_element(:id, "add_to_faculty_journal").should_not be_displayed
+    end
+
+    it "should not allow user notes to teachers" do
+      @course.account.update_attribute :enable_user_notes, true
+      new_conversation
+      add_recipient("teacher bob")
+      driver.find_element(:id, "add_to_faculty_journal").should_not be_displayed
+    end
+
+    it "should not allow user notes on group conversations" do
+      @course.account.update_attribute :enable_user_notes, true
+      new_conversation
+      add_recipient("student bob")
+      add_recipient("teacher bob")
+      driver.find_element(:id, "add_to_faculty_journal").should_not be_displayed
+      find_with_jquery("#create_message_form input:visible").send_keys :backspace
+      driver.find_element(:id, "add_to_faculty_journal").should be_displayed
+    end
+
+    it "should allow user notes on new private conversations with students" do
+      @course.account.update_attribute :enable_user_notes, true
+      new_conversation
+      add_recipient("student bob")
+      checkbox = driver.find_element(:id, "add_to_faculty_journal")
+      checkbox.should be_displayed
+      checkbox.click
+      submit_message_form(:add_recipient => false)
+      @the_student.user_notes.size.should eql(1)
+    end
+
+    it "should allow user notes on existing private conversations with students" do
+      @course.account.update_attribute :enable_user_notes, true
+      new_conversation
+      add_recipient("student bob")
+      submit_message_form(:add_recipient => false)
+      checkbox = driver.find_element(:id, "add_to_faculty_journal")
+      checkbox.should be_displayed
+      checkbox.click
+      submit_message_form
+      @the_student.user_notes.size.should eql(1)
+    end
+  end
 end
 
 describe "conversations Windows-Firefox-Tests" do