ConversationsController#toggle_readwrite_status is not

ConversationsController#update.

app/controllers/conversations_controller.rb

@@ -6,7 +6,7 @@ def ssl_required?
   before_filter :login_or_oauth_required,
     :except => [:index, :show, :auto_complete_for_conversation_name, :complete_name]
   before_filter :find_conversation,
-    :except => [:bookmarked, :complete_name, :create, :spawn, :new, :index, :new_messages, :toggle_readwrite_status]
+    :except => [:bookmarked, :complete_name, :create, :spawn, :new, :index, :new_messages, :update]
   before_filter :check_read_access, :only => [:show]
   after_filter :store_location, :only => [:show, :new]
 
@@ -133,12 +133,13 @@ def follow_email_with_token
   end
 
   #----------------------------------------------------------------------------
-  def toggle_readwrite_status
+  def update
     @conversation = Conversation.find(params[:id])
-    read_only = (params[:mode] == 'rw') ? false : true
-    @conversation.update_attributes( :read_only => read_only ) if ( @conversation.owner == current_user )
+    @conversation.toggle_read_only_status if current_user.owns?(@conversation) 
     Rails.cache.write("conversation_#{params[:id]}", @conversation)
-    redirect_to conversation_path( @conversation )
+    respond_to do |format|
+      format.html { redirect_to(conversation_path(@conversation)) }
+    end
   end
 
   #----------------------------------------------------------------------------

app/models/conversation.rb

@@ -102,6 +102,11 @@ def spawned?
     !self.parent_message_id.nil?
   end
 
+  def toggle_read_only_status
+    toggled_status = !self.read_only?
+    self.update_attribute(:read_only, toggled_status)
+  end
+
   def add_visit(user)
     if cv = ConversationVisit.find_by_user_id_and_conversation_id(user.id, self.id)
       cv.increment!( :visits_count ) 

app/views/layouts/messages.html.erb

@@ -206,11 +206,7 @@
                         <div id="m2" 
                             onmouseover="mcancelclosetime()" 
                             onmouseout="mclosetime()">
-                            <% if @conversation.read_only %>
-                    	        <%= link_to t("ui.make_writable"), toggle_readwrite_status_conversation_path(@conversation, :mode => 'rw'), :method => 'put' %>
-                    	      <% else %>
-                    	        <%= link_to t("ui.make_read_only"), toggle_readwrite_status_conversation_path(@conversation), :method => 'put' %>
-                    	      <% end %>
+                          <%= link_to(@conversation.read_only? ? t("ui.make_writable") : t("ui.make_read_only"), conversation_path(@conversation), :method => 'put') %>
                         </div>
                     </li>
                     <% end %>

app/views/subscribers/index.html.erb

@@ -12,13 +12,9 @@
     	    <span class="sbutton round"><%= link_to t("ui.back_to_parent"), conversation_path(@conversation.parent_message.conversation)%></span>
     	  <% end %>
         <span class="sbutton round"><%= render :partial => 'subscriptions/follow', :locals => { :conversation => @conversation } %></span>
-        <% if current_user == @conversation.owner %>
+        <% if current_user.owns?(@conversation) %>
     	    <span class="sbutton round">
-    	      <% if @conversation.read_only %>
-    	        <%= link_to t("ui.make_writable"), toggle_readwrite_status_conversation_path(@conversation, :mode => 'rw'), :method => 'put' %>
-    	      <% else %>
-    	        <%= link_to t("ui.make_read_only"), toggle_readwrite_status_conversation_path(@conversation), :method => 'put' %>
-    	      <% end %>
+            <%= link_to(@conversation.read_only? ? t("ui.make_writable") : t("ui.make_read_only"), conversation_path(@conversation), :method => 'put') %>
     	    </span>
     	  <% end %>
 

config/routes.rb

@@ -21,7 +21,6 @@
   end
 
   map.resources :conversations, :collection => {:bookmarked => :get, :new_messages => :get}, :member => {
-    :toggle_readwrite_status => :put,
     :toggle_bookmark         => :post,
     :follow_with_token       => :get,
     :follow_email_with_token => :get,

test/functional/conversations_controller_test.rb

@@ -217,7 +217,7 @@ def setup
     end
   end # context new action
 
-  context "toggle_readwrite_status action" do
+  context "update action" do
     setup do
       @owner = Factory.create(:user, :login => 'user1')
       @conversation = Factory.create(:conversation, :user => @current_user)
@@ -231,28 +231,30 @@ def setup
       should "only allow changes if the current_user is the conversation owner" do
         # try to change to writeable
         @other_conversation.update_attribute(:read_only, true)
-        put :toggle_readwrite_status, :id => @other_conversation, :mode => 'rw'
-        assert_equal true, assigns(:conversation).read_only
+        put :update, :id => @other_conversation
+        assert_equal true, assigns(:conversation).read_only?
 
         # try to change to readonly
         @other_conversation.update_attribute(:read_only, false)
-        put :toggle_readwrite_status, :id => @other_conversation
-        assert_equal false, assigns(:conversation).read_only
+        put :update, :id => @other_conversation
+        assert_equal false, assigns(:conversation).read_only?
       end
     end
 
     context "convo belongs to current_user" do
-      should "make readonly with no mode param" do
-        put :toggle_readwrite_status, :id => @conversation
-        assert_equal true, assigns(:conversation).read_only
+      should "make readonly when conversation is writable" do
+        put :update, :id => @conversation
+        assert_equal true, assigns(:conversation).read_only?
       end
 
-      should "make writeable with rw mode param" do
-        put :toggle_readwrite_status, :id => @conversation, :mode => 'rw'
-        assert_equal false, assigns(:conversation).read_only 
+      should "make writeable when conversation is readonly" do
+        @conversation.update_attribute(:read_only, true)
+
+        put :update, :id => @conversation
+        assert_equal false, assigns(:conversation).read_only?
       end
     end
-  end # context toggle_readwrite_status action
+  end # context update action
 
   context "show action" do
     setup do

test/unit/conversation_test.rb

@@ -178,6 +178,11 @@ class ConversationTest < ActiveSupport::TestCase
     should "not be writable by the users" do
       assert !@conversation.writable_by?(@user2)
     end
+
+    should "become writable on read only status toggle" do
+      @conversation.toggle_read_only_status
+      assert !@conversation.read_only?
+    end
   end
 
   context "A writable conversation" do
@@ -196,6 +201,11 @@ class ConversationTest < ActiveSupport::TestCase
     should "be writable by the users" do
       assert @conversation.writable_by?(@user2)
     end
+
+    should "become read only on read only status toggle" do
+      @conversation.toggle_read_only_status
+      assert @conversation.read_only?
+    end
   end
 
   context "A visit to a conversation" do