Skip to content

Commit 2e54ebe

Browse files
committed
friendlier conversation urls/redirects
Change-Id: I85ea47245d72149f984717c169ed855813da8532 Reviewed-on: https://gerrit.instructure.com/5339 Tested-by: Hudson <hudson@instructure.com> Reviewed-by: Jacob Fugal <jacob@instructure.com>
1 parent 443d925 commit 2e54ebe

10 files changed

Lines changed: 20 additions & 9 deletions

app/controllers/conversations_controller.rb

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,8 @@ def create
9999
end
100100

101101
def show
102+
return redirect_to "/conversations/#/conversations/#{@conversation.conversation_id}" unless request.xhr?
103+
102104
@conversation.mark_as_read! if @conversation.unread?
103105
submissions = []
104106
if @conversation.one_on_one?
@@ -284,6 +286,7 @@ def matching_participants(options)
284286

285287
def get_conversation(allow_deleted = false)
286288
@conversation = (allow_deleted ? @current_user.all_conversations : @current_user.conversations).find_by_conversation_id(params[:id] || params[:conversation_id] || 0)
289+
raise ActiveRecord::RecordNotFound unless @conversation
287290
end
288291

289292
def create_message_on_conversation(conversation=@conversation, update_for_sender=true)

app/messages/added_to_conversation.email.erb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
<% define_content :link do %>
2-
http://<%= HostUrl.context_host(asset.context) %>/conversations/#/conversations/<%= asset.conversation_id %>
2+
http://<%= HostUrl.context_host(asset.context) %>/conversations/<%= asset.conversation_id %>
33
<% end %>
44
<% define_content :user_name do %>
55
<%= asset.author.short_name rescue t(:unknown_user, "Unknown User") %>
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
<% define_content :link do %>
2-
http://<%= HostUrl.context_host(asset.context) %>/conversations/#/conversations/<%= asset.conversation_id %>
2+
http://<%= HostUrl.context_host(asset.context) %>/conversations/<%= asset.conversation_id %>
33
<% end %>
44
<%= t :body, "%{user_name} just added you to a conversation in Canvas.", :user_name => (asset.author.short_name rescue t(:unknown_user, "Unknown User")) %><br/>
55
<b><a href="<%= content :link %>"><%= t :link_text, "View Conversation" %></a></b>

app/messages/added_to_conversation.summary.erb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
<% define_content :link do %>
2-
http://<%= HostUrl.context_host(asset.context) %>/conversations/#/conversations/<%= asset.conversation_id %>
2+
http://<%= HostUrl.context_host(asset.context) %>/conversations/<%= asset.conversation_id %>
33
<% end %>
44
<% define_content :user_name do %>
55
<%= asset.author.short_name rescue t(:unknown_user, "Unknown User") %>

app/messages/conversation_message.email.erb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
<% define_content :link do %>
2-
http://<%= HostUrl.context_host(asset.context) %>/conversations/#/conversations/<%= asset.conversation_id %>
2+
http://<%= HostUrl.context_host(asset.context) %>/conversations/<%= asset.conversation_id %>
33
<% end %>
44
<% define_content :user_name do %>
55
<%= asset.author.short_name rescue t(:unknown_user, "Unknown User") %>
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
<% define_content :link do %>
2-
http://<%= HostUrl.context_host(asset.context) %>/conversations/#/conversations/<%= asset.conversation_id %>
2+
http://<%= HostUrl.context_host(asset.context) %>/conversations/<%= asset.conversation_id %>
33
<% end %>
44
<%= t :body, "%{user_name} just sent you a message in Canvas.", :user_name => (asset.author.short_name rescue t(:unknown_user, "Unknown User")) %><br/>
55
<b><a href="<%= content :link %>"><%= t :link_text, "View Message" %></a></b>

app/messages/conversation_message.summary.erb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
<% define_content :link do %>
2-
http://<%= HostUrl.context_host(asset.context) %>/conversations/#/conversations/<%= asset.conversation_id %>
2+
http://<%= HostUrl.context_host(asset.context) %>/conversations/<%= asset.conversation_id %>
33
<% end %>
44
<% define_content :user_name do %>
55
<%= asset.author.short_name rescue t(:unknown_user, "Unknown User") %>

app/views/context/_dashboard_conversation.html.erb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
<% conversation = dashboard_conversation %>
2-
<% conversation_path = "#{conversations_path}#/conversations/#{conversation ? conversation.id : '{{ id }}'}" %>
2+
<% conversation_path = conversation_path(conversation ? conversation.id : '{{ id }}') %>
33
<div class="communication_message conversation message_no_context" style="<%= hidden unless conversation %>" id="conversation_<%= conversation ? conversation.id : "blank" %>">
44
<%= render :partial => 'context/dashboard_ignore_link', :object => stream_item %>
55
<% if conversation %><a name="conversation_<%= conversation.id %>"></a><% end %>

app/views/context/_dashboard_conversation_message.html.erb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
<% message = dashboard_conversation_message || nil %>
22
<% hide_message ||= false %>
33
<% author = message ? message.author : @current_user %>
4-
<% conversation_path = "#{conversations_path}#/conversations/#{conversation ? conversation.id : '{{ id }}'}" %>
4+
<% conversation_path = conversation_path(conversation ? conversation.id : '{{ id }}') %>
55
<div id="dashboard_conversation_message_<%= message ? message.id : "blank" %>" style="<%= hidden unless message %>" class="communication_sub_message <%= 'toggled_communication_sub_message' if hide_message %> <%= 'blank' unless message %>">
66
<div class="header">
77
<div class="header_title">

spec/controllers/conversations_controller_spec.rb

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,11 +62,19 @@ def conversation(num_other_users = 1)
6262
end
6363

6464
describe "GET 'show'" do
65-
it "should assign variables" do
65+
it "should redirect if not xhr" do
6666
course_with_student_logged_in(:active_all => true)
6767
conversation
6868

6969
get 'show', :id => @conversation.conversation_id
70+
response.should be_redirect
71+
end
72+
73+
it "should assign variables" do
74+
course_with_student_logged_in(:active_all => true)
75+
conversation
76+
77+
xhr :get, 'show', :id => @conversation.conversation_id
7078
response.should be_success
7179
assigns[:conversation].should == @conversation
7280
end

0 commit comments

Comments
 (0)