Skip to content

Commit c8c15dc

Browse files
jensengzwily
authored andcommitted
fix faculty journal checks
Change-Id: I23692f617b6949cfc13eed3e6c1046b0c723c0f5 Reviewed-on: https://gerrit.instructure.com/5206 Tested-by: Hudson <hudson@instructure.com> Reviewed-by: Jacob Fugal <jacob@instructure.com>
1 parent c2c7626 commit c8c15dc

7 files changed

Lines changed: 49 additions & 10 deletions

File tree

app/coffeescripts/messages.coffee

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -541,7 +541,7 @@ I18n.scoped 'conversations', (I18n) ->
541541
$form.attr action: $selected_conversation.find('a.details_link').attr('add_url')
542542

543543
reset_message_form()
544-
$form.find('#user_note_info').showIf($selected_conversation?.hasClass('private')).find('input').attr('checked', false)
544+
$form.find('#user_note_info').hide().find('input').attr('checked', false)
545545
$form.show().find(':input:visible:first').focus()
546546

547547
reset_message_form = ->
@@ -610,6 +610,13 @@ I18n.scoped 'conversations', (I18n) ->
610610
for user in data.participants when !MessageInbox.user_cache[user.id]?.avatar
611611
MessageInbox.user_cache[user.id] = user
612612
user.html_name = html_name_for_user(user)
613+
$form.find('#user_note_info').showIf(
614+
$c.hasClass('private') and
615+
(user_id = $c.find('.participant').first().data('id')) and
616+
(user = MessageInbox.user_cache[user_id]) and
617+
can_add_notes_for(user)
618+
)
619+
inbox_resize()
613620
$messages.show()
614621
i = j = 0
615622
message = data.messages[0]
@@ -656,9 +663,10 @@ I18n.scoped 'conversations', (I18n) ->
656663
$.htmlEscape(user.name) + if shared_contexts.length then " <em>" + $.htmlEscape(shared_contexts) + "</em>" else ''
657664

658665
can_add_notes_for = (user) ->
666+
return false unless MessageInbox.notes_enabled
659667
return true if user.can_add_notes
660668
for course_id, roles of user.common_courses
661-
return true if 'StudentEnrollment' in roles and (MessageInbox.can_add_notes or MessageInbox.contexts.courses[course_id]?.can_add_notes)
669+
return true if 'StudentEnrollment' in roles and (MessageInbox.can_add_notes_for_account or MessageInbox.contexts.courses[course_id]?.can_add_notes)
662670
false
663671

664672
formatted_message = (message) ->
@@ -1423,6 +1431,7 @@ I18n.scoped 'conversations', (I18n) ->
14231431
$form.find('#group_conversation').attr('checked', true)
14241432
$form.find('#group_conversation_info').hide()
14251433
$form.find('#user_note_info').showIf((user = MessageInbox.user_cache[tokens[0]]) and can_add_notes_for(user))
1434+
inbox_resize()
14261435

14271436
$(window).resize inbox_resize
14281437
setTimeout inbox_resize

app/helpers/application_helper.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,7 @@ def context_url(context, *opts)
204204

205205
def message_user_path(user)
206206
params = {:user_id => user.id, :user_name => user.short_name}
207-
params[:can_add_notes] = true if user.grants_right?(@current_user, :create_user_notes)
207+
params[:can_add_notes] = true if user.grants_right?(@current_user, :create_user_notes) && user.associated_accounts.any?{|a| a.enable_user_notes }
208208
conversations_path + "#/conversations?" + params.to_query.gsub('+', '%20')
209209
end
210210

app/models/conversation_message.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ def format_event_message
125125
def generate_user_note
126126
return unless recipients.size == 1
127127
recipient = recipients.first
128-
return unless recipient.grants_right?(author, :create_user_notes)
128+
return unless recipient.grants_right?(author, :create_user_notes) && recipient.associated_accounts.any?{|a| a.enable_user_notes }
129129

130130
self.extend TextHelper
131131
title = t(:subject, "Private message, %{timestamp}", :timestamp => date_string(created_at))

app/views/conversations/index.html.erb

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -222,7 +222,9 @@
222222
MessageInbox.initial_conversations_count = <%= @conversations_count %>;
223223
MessageInbox.scope = <%= raw @scope.to_json %>;
224224
MessageInbox.label_scope = <%= raw @label.to_json %>;
225-
MessageInbox.can_add_notes = <%= raw @current_user.associated_accounts.any?{|a| a.enable_user_notes && a.grants_right?(@current_user, nil, :manage_students) } %>;
225+
<% # TODO: get note-enabled-ness per account, and cross reference with the recipient's associated account ids. this is already enforced in the model %>
226+
MessageInbox.notes_enabled = <%= raw @current_user.associated_accounts.any?{|a| a.enable_user_notes } %>;
227+
MessageInbox.can_add_notes_for_account = MessageInbox.notes_enabled && <%= raw @current_user.associated_accounts.any?{|a| a.grants_right?(@current_user, nil, :manage_students) } %>;
226228
<% unless @current_user.watched_conversations_intro? %>
227229
$.conversationsIntroSlideshow()
228230
<% end %>

public/javascripts/compiled/messages.js

Lines changed: 11 additions & 5 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

spec/controllers/conversations_controller_spec.rb

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ def conversation(num_other_users = 1)
2525
enrollment = @course.enroll_student(u)
2626
enrollment.workflow_state = 'active'
2727
enrollment.save
28+
u.associated_accounts << Account.default
2829
u.id
2930
}
3031
@conversation = @user.initiate_conversation(user_ids)
@@ -174,7 +175,9 @@ def conversation(num_other_users = 1)
174175
end
175176

176177
it "should generate a user note when requested" do
178+
Account.default.update_attribute :enable_user_notes, true
177179
course_with_teacher_logged_in(:active_all => true)
180+
@teacher.associated_accounts << Account.default
178181
conversation
179182

180183
post 'add_message', :conversation_id => @conversation.conversation_id, :body => "hello world"

spec/models/conversation_message_spec.rb

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,8 +109,11 @@ def add_last_student
109109

110110
context "generate_user_note" do
111111
it "should add a user note under nominal circumstances" do
112+
Account.default.update_attribute :enable_user_notes, true
112113
course_with_teacher
114+
@teacher.associated_accounts << Account.default
113115
student = student_in_course.user
116+
student.associated_accounts << Account.default
114117
conversation = @teacher.initiate_conversation([student.id])
115118
message = conversation.add_message("reprimanded!")
116119
message.created_at = Time.at(0) # Jan 1, 1970 00:00:00 UTC
@@ -122,10 +125,26 @@ def add_last_student
122125
note.note.should eql("reprimanded!")
123126
end
124127

128+
it "should fail if notes are disabled on the account" do
129+
Account.default.update_attribute :enable_user_notes, false
130+
course_with_teacher
131+
@teacher.associated_accounts << Account.default
132+
student = student_in_course.user
133+
student.associated_accounts << Account.default
134+
conversation = @teacher.initiate_conversation([student.id])
135+
message = conversation.add_message("reprimanded!")
136+
message.generate_user_note.should be_nil
137+
student.user_notes.size.should be(0)
138+
end
139+
125140
it "should fail if there's more than one recipient" do
141+
Account.default.update_attribute :enable_user_notes, true
126142
course_with_teacher
143+
@teacher.associated_accounts << Account.default
127144
student1 = student_in_course.user
145+
student1.associated_accounts << Account.default
128146
student2 = student_in_course.user
147+
student2.associated_accounts << Account.default
129148
conversation = @teacher.initiate_conversation([student1.id, student2.id])
130149
message = conversation.add_message("message")
131150
message.generate_user_note.should be_nil

0 commit comments

Comments
 (0)