Refactor diary entries to use create and update methods

This brings it slightly more into line with resourceful routing.

app/abilities/ability.rb

@@ -36,7 +36,7 @@ class Ability
 
       if Settings.status != "database_offline"
         can [:index, :new, :create, :show, :edit, :update, :destroy], ClientApplication
-        can [:create, :edit, :comment, :subscribe, :unsubscribe], DiaryEntry
+        can [:new, :create, :edit, :update, :comment, :subscribe, :unsubscribe], DiaryEntry
         can [:new, :create, :reply, :show, :inbox, :outbox, :mark, :destroy], Message
         can [:close, :reopen], Note
         can [:new, :create], Report

app/controllers/diary_entries_controller.rb

@@ -8,38 +8,40 @@ class DiaryEntriesController < ApplicationController
   authorize_resource
 
   before_action :lookup_user, :only => [:show, :comments]
-  before_action :check_database_writable, :only => [:new, :edit, :comment, :hide, :hidecomment, :subscribe, :unsubscribe]
-  before_action :allow_thirdparty_images, :only => [:new, :edit, :index, :show, :comments]
+  before_action :check_database_writable, :only => [:new, :create, :edit, :update, :comment, :hide, :hidecomment, :subscribe, :unsubscribe]
+  before_action :allow_thirdparty_images, :only => [:new, :create, :edit, :update, :index, :show, :comments]
 
   def new
     @title = t "diary_entries.new.title"
 
-    if request.post?
-      @diary_entry = DiaryEntry.new(entry_params)
-      @diary_entry.user = current_user
+    default_lang = current_user.preferences.where(:k => "diary.default_language").first
+    lang_code = default_lang ? default_lang.v : current_user.preferred_language
+    @diary_entry = DiaryEntry.new(entry_params.merge(:language_code => lang_code))
+    set_map_location
+    render :action => "new"
+  end
 
-      if @diary_entry.save
-        default_lang = current_user.preferences.where(:k => "diary.default_language").first
-        if default_lang
-          default_lang.v = @diary_entry.language_code
-          default_lang.save!
-        else
-          current_user.preferences.create(:k => "diary.default_language", :v => @diary_entry.language_code)
-        end
+  def create
+    @title = t "diary_entries.new.title"
 
-        # Subscribe user to diary comments
-        @diary_entry.subscriptions.create(:user => current_user)
+    @diary_entry = DiaryEntry.new(entry_params)
+    @diary_entry.user = current_user
 
-        redirect_to :action => "index", :display_name => current_user.display_name
-      else
-        render :action => "edit"
-      end
-    else
+    if @diary_entry.save
       default_lang = current_user.preferences.where(:k => "diary.default_language").first
-      lang_code = default_lang ? default_lang.v : current_user.preferred_language
-      @diary_entry = DiaryEntry.new(entry_params.merge(:language_code => lang_code))
-      set_map_location
-      render :action => "edit"
+      if default_lang
+        default_lang.v = @diary_entry.language_code
+        default_lang.save!
+      else
+        current_user.preferences.create(:k => "diary.default_language", :v => @diary_entry.language_code)
+      end
+
+      # Subscribe user to diary comments
+      @diary_entry.subscriptions.create(:user => current_user)
+
+      redirect_to :action => "index", :display_name => current_user.display_name
+    else
+      render :action => "new"
     end
   end
 
@@ -47,13 +49,25 @@ class DiaryEntriesController < ApplicationController
     @title = t "diary_entries.edit.title"
     @diary_entry = DiaryEntry.find(params[:id])
 
+    redirect_to diary_entry_path(@diary_entry.user, @diary_entry) if current_user != @diary_entry.user
+
+    set_map_location
+  rescue ActiveRecord::RecordNotFound
+    render :action => "no_such_entry", :status => :not_found
+  end
+
+  def update
+    @title = t "diary_entries.edit.title"
+    @diary_entry = DiaryEntry.find(params[:id])
+
     if current_user != @diary_entry.user
       redirect_to diary_entry_path(@diary_entry.user, @diary_entry)
     elsif params[:diary_entry] && @diary_entry.update(entry_params)
       redirect_to diary_entry_path(@diary_entry.user, @diary_entry)
+    else
+      set_map_location
+      render :action => "edit"
     end
-
-    set_map_location
   rescue ActiveRecord::RecordNotFound
     render :action => "no_such_entry", :status => :not_found
   end

app/views/diary_entries/edit.html.erb

@@ -8,7 +8,7 @@
 
 <%= error_messages_for "diary_entry" %>
 
-<%= form_for :diary_entry do |f| %>
+<%= form_for @diary_entry, :url => diary_entry_path(current_user, @diary_entry), :html => { :method => :put } do |f| %>
   <div class="diary_entry standard-form">
     <fieldset>
       <div class='form-row'>
@@ -42,10 +42,6 @@
       </div>
     </fieldset>
 
-    <% if action_name == 'new' %>
-      <%= submit_tag t("diary_entries.new.publish_button") %>
-    <% else %>
-      <%= submit_tag t(".save_button") %>
-    <% end %>
+    <%= submit_tag t(".save_button") %>
   </div>
 <% end %>

app/views/diary_entries/index.html.erb

@@ -13,13 +13,13 @@
       <% if @user %>
         <% if @user == current_user %>
           <div>
-            <li><%= link_to image_tag("new.png", :class => "small_icon", :border => 0) + t(".new"), diary_new_path, :title => t(".new_title") %></li>
+            <li><%= link_to image_tag("new.png", :class => "small_icon", :border => 0) + t(".new"), new_diary_entry_path, :title => t(".new_title") %></li>
           </div>
         <% end %>
       <% else %>
         <% if current_user %>
           <div>
-            <li><%= link_to image_tag("new.png", :class => "small_icon", :border => 0) + t(".new"), diary_new_path, :title => t(".new_title") %></li>
+            <li><%= link_to image_tag("new.png", :class => "small_icon", :border => 0) + t(".new"), new_diary_entry_path, :title => t(".new_title") %></li>
           </div>
         <% end %>
       <% end %>

app/views/diary_entries/new.html.erb

@@ -0,0 +1,47 @@
+<% content_for :head do %>
+  <%= javascript_include_tag "diary_entry" %>
+<% end %>
+
+<% content_for :heading do %>
+  <h1><%= @title %></h1>
+<% end %>
+
+<%= error_messages_for "diary_entry" %>
+
+<%= form_for @diary_entry do |f| %>
+  <div class="diary_entry standard-form">
+    <fieldset>
+      <div class='form-row'>
+        <label class="standard-label"><%= t ".subject" -%></label>
+        <%= f.text_field :title, :class => "richtext_title" %>
+      </div>
+      <div class='form-row'>
+        <label class="standard-label"><%= t ".body" -%></label>
+        <%= richtext_area :diary_entry, :body, :cols => 80, :rows => 20, :format => @diary_entry.body_format %>
+      </div>
+      <div class='form-row'>
+        <label class="standard-label"><%= t ".language" -%></label>
+        <%= f.collection_select :language_code, Language.order(:english_name), :code, :name %>
+    </div>
+    </fieldset>
+    <fieldset class='location'>
+      <label class="standard-label"><%= t ".location" -%></label>
+      <%= content_tag "div", "", :id => "map", :data => { :lat => @lat, :lon => @lon, :zoom => @zoom } %>
+      <div class='form-row clearfix'>
+        <div class='form-column'>
+          <label class="secondary standard-label"><%= t ".latitude" -%></label>
+          <%= f.text_field :latitude, :size => 20, :id => "latitude" %>
+        </div>
+        <div class='form-column'>
+          <label class="secondary standard-label"><%= t ".longitude" -%></label>
+          <%= f.text_field :longitude, :size => 20, :id => "longitude" %>
+        </div>
+        <div class='form-column'>
+          <a href="#" id="usemap"><%= t ".use_map_link" -%></a>
+        </div>
+      </div>
+    </fieldset>
+
+    <%= submit_tag t("diary_entries.new.publish_button") %>
+  </div>
+<% end %>

config/locales/en.yml

@@ -282,6 +282,14 @@ en:
   diary_entries:
     new:
       title: New Diary Entry
+      subject: "Subject:"
+      body: "Body:"
+      language: "Language:"
+      location: "Location:"
+      latitude: "Latitude:"
+      longitude: "Longitude:"
+      use_map_link: "use map"
+      marker_text: Diary entry location
       publish_button: "Publish"
     index:
       title: "Users' diaries"

config/routes.rb

@@ -216,7 +216,7 @@ OpenStreetMap::Application.routes.draw do
   post "/trace/:id/delete" => "traces#delete", :id => /\d+/
 
   # diary pages
-  match "/diary/new" => "diary_entries#new", :via => [:get, :post]
+  resources :diary_entries, :path => "diary", :only => [:new, :create]
   get "/diary/friends" => "diary_entries#index", :friends => true, :as => "friend_diaries"
   get "/diary/nearby" => "diary_entries#index", :nearby => true, :as => "nearby_diaries"
   get "/user/:display_name/diary/rss" => "diary_entries#rss", :defaults => { :format => :rss }
@@ -227,9 +227,10 @@ OpenStreetMap::Application.routes.draw do
   get "/user/:display_name/diary" => "diary_entries#index"
   get "/diary/:language" => "diary_entries#index"
   get "/diary" => "diary_entries#index"
-  get "/user/:display_name/diary/:id" => "diary_entries#show", :id => /\d+/, :as => :diary_entry
+  scope "/user/:display_name" do
+    resources :diary_entries, :path => "diary", :only => [:edit, :update, :show]
+  end
   post "/user/:display_name/diary/:id/newcomment" => "diary_entries#comment", :id => /\d+/
-  match "/user/:display_name/diary/:id/edit" => "diary_entries#edit", :via => [:get, :post], :id => /\d+/
   post "/user/:display_name/diary/:id/hide" => "diary_entries#hide", :id => /\d+/, :as => :hide_diary_entry
   post "/user/:display_name/diary/:id/hidecomment/:comment" => "diary_entries#hidecomment", :id => /\d+/, :comment => /\d+/, :as => :hide_diary_comment
   post "/user/:display_name/diary/:id/subscribe" => "diary_entries#subscribe", :as => :diary_entry_subscribe, :id => /\d+/

test/controllers/diary_entries_controller_test.rb

@@ -62,8 +62,8 @@ class DiaryEntriesControllerTest < ActionController::TestCase
       { :controller => "diary_entries", :action => "new" }
     )
     assert_routing(
-      { :path => "/diary/new", :method => :post },
-      { :controller => "diary_entries", :action => "new" }
+      { :path => "/diary", :method => :post },
+      { :controller => "diary_entries", :action => "create" }
     )
     assert_routing(
       { :path => "/user/username/diary/1", :method => :get },
@@ -74,8 +74,8 @@ class DiaryEntriesControllerTest < ActionController::TestCase
       { :controller => "diary_entries", :action => "edit", :display_name => "username", :id => "1" }
     )
     assert_routing(
-      { :path => "/user/username/diary/1/edit", :method => :post },
-      { :controller => "diary_entries", :action => "edit", :display_name => "username", :id => "1" }
+      { :path => "/user/username/diary/1", :method => :put },
+      { :controller => "diary_entries", :action => "update", :display_name => "username", :id => "1" }
     )
     assert_routing(
       { :path => "/user/username/diary/1/newcomment", :method => :post },
@@ -116,7 +116,7 @@ class DiaryEntriesControllerTest < ActionController::TestCase
       assert_select "h1", :text => /New Diary Entry/, :count => 1
     end
     assert_select "div#content", :count => 1 do
-      assert_select "form[action='/diary/new'][method=post]", :count => 1 do
+      assert_select "form[action='/diary'][method=post]", :count => 1 do
         assert_select "input#diary_entry_title[name='diary_entry[title]']", :count => 1
         assert_select "textarea#diary_entry_body[name='diary_entry[body]']", :text => "", :count => 1
         assert_select "select#diary_entry_language_code", :count => 1
@@ -140,30 +140,30 @@ class DiaryEntriesControllerTest < ActionController::TestCase
           :session => { :user => create(:user).id }
     end
     assert_response :success
-    assert_template :edit
+    assert_template :new
   end
 
-  def test_new_no_body
+  def test_create_no_body
     # Now try creating a invalid diary entry with an empty body
     user = create(:user)
     assert_no_difference "DiaryEntry.count" do
-      post :new,
+      post :create,
            :params => { :commit => "save",
                         :diary_entry => { :title => "New Title", :body => "", :latitude => "1.1",
                                           :longitude => "2.2", :language_code => "en" } },
            :session => { :user => user.id }
     end
     assert_response :success
-    assert_template :edit
+    assert_template :new
 
     assert_nil UserPreference.where(:user_id => user.id, :k => "diary.default_language").first
   end
 
-  def test_new_post
+  def test_create
     # Now try creating a diary entry
     user = create(:user)
     assert_difference "DiaryEntry.count", 1 do
-      post :new,
+      post :create,
            :params => { :commit => "save",
                         :diary_entry => { :title => "New Title", :body => "This is a new body for the diary entry", :latitude => "1.1",
                                           :longitude => "2.2", :language_code => "en" } },
@@ -185,13 +185,13 @@ class DiaryEntriesControllerTest < ActionController::TestCase
     assert_equal "en", UserPreference.where(:user_id => user.id, :k => "diary.default_language").first.v
   end
 
-  def test_new_german
+  def test_create_german
     create(:language, :code => "de")
     user = create(:user)
 
     # Now try creating a diary entry in a different language
     assert_difference "DiaryEntry.count", 1 do
-      post :new,
+      post :create,
            :params => { :commit => "save",
                         :diary_entry => { :title => "New Title", :body => "This is a new body for the diary entry", :latitude => "1.1",
                                           :longitude => "2.2", :language_code => "de" } },
@@ -221,7 +221,7 @@ class DiaryEntriesControllerTest < ActionController::TestCase
 
     # Try creating a spammy diary entry
     assert_difference "DiaryEntry.count", 1 do
-      post :new,
+      post :create,
            :params => { :commit => "save",
                         :diary_entry => { :title => spammy_title, :body => spammy_body, :language_code => "en" } },
            :session => { :user => user.id }
@@ -284,7 +284,7 @@ class DiaryEntriesControllerTest < ActionController::TestCase
       assert_select "h1", :text => /Edit diary entry/, :count => 1
     end
     assert_select "div#content", :count => 1 do
-      assert_select "form[action='/user/#{ERB::Util.u(entry.user.display_name)}/diary/#{entry.id}/edit'][method=post]", :count => 1 do
+      assert_select "form[action='/user/#{ERB::Util.u(entry.user.display_name)}/diary/#{entry.id}'][method=post]", :count => 1 do
         assert_select "input#diary_entry_title[name='diary_entry[title]'][value='#{entry.title}']", :count => 1
         assert_select "textarea#diary_entry_body[name='diary_entry[body]']", :text => entry.body, :count => 1
         assert_select "select#diary_entry_language_code", :count => 1
@@ -293,7 +293,7 @@ class DiaryEntriesControllerTest < ActionController::TestCase
         assert_select "input[name=commit][type=submit][value=Save]", :count => 1
         assert_select "input[name=commit][type=submit][value=Edit]", :count => 1
         assert_select "input[name=commit][type=submit][value=Preview]", :count => 1
-        assert_select "input", :count => 7
+        assert_select "input", :count => 8
       end
     end
 
@@ -303,11 +303,11 @@ class DiaryEntriesControllerTest < ActionController::TestCase
     new_latitude = "1.1"
     new_longitude = "2.2"
     new_language_code = "en"
-    post :edit,
-         :params => { :display_name => entry.user.display_name, :id => entry.id, :commit => "save",
-                      :diary_entry => { :title => new_title, :body => new_body, :latitude => new_latitude,
-                                        :longitude => new_longitude, :language_code => new_language_code } },
-         :session => { :user => entry.user.id }
+    put :update,
+        :params => { :display_name => entry.user.display_name, :id => entry.id, :commit => "save",
+                     :diary_entry => { :title => new_title, :body => new_body, :latitude => new_latitude,
+                                       :longitude => new_longitude, :language_code => new_language_code } },
+        :session => { :user => entry.user.id }
     assert_response :redirect
     assert_redirected_to :action => :show, :display_name => entry.user.display_name, :id => entry.id
 

test/controllers/users_controller_test.rb

@@ -445,34 +445,34 @@ class UsersControllerTest < ActionController::TestCase
   def test_confirm_success_no_token_with_referer
     user = create(:user, :pending)
     stub_gravatar_request(user.email)
-    confirm_string = user.tokens.create(:referer => diary_new_path).token
+    confirm_string = user.tokens.create(:referer => new_diary_entry_path).token
 
     @request.cookies["_osm_session"] = user.display_name
     post :confirm, :params => { :display_name => user.display_name, :confirm_string => confirm_string }
-    assert_redirected_to login_path(:referer => diary_new_path)
+    assert_redirected_to login_path(:referer => new_diary_entry_path)
     assert_match(/Confirmed your account/, flash[:notice])
   end
 
   def test_confirm_success_good_token_with_referer
     user = create(:user, :pending)
     stub_gravatar_request(user.email)
-    confirm_string = user.tokens.create(:referer => diary_new_path).token
+    confirm_string = user.tokens.create(:referer => new_diary_entry_path).token
     token = user.tokens.create.token
 
     @request.cookies["_osm_session"] = user.display_name
     post :confirm, :params => { :display_name => user.display_name, :confirm_string => confirm_string }, :session => { :token => token }
-    assert_redirected_to diary_new_path
+    assert_redirected_to new_diary_entry_path
   end
 
   def test_confirm_success_bad_token_with_referer
     user = create(:user, :pending)
     stub_gravatar_request(user.email)
-    confirm_string = user.tokens.create(:referer => diary_new_path).token
+    confirm_string = user.tokens.create(:referer => new_diary_entry_path).token
     token = create(:user).tokens.create.token
 
     @request.cookies["_osm_session"] = user.display_name
     post :confirm, :params => { :display_name => user.display_name, :confirm_string => confirm_string }, :session => { :token => token }
-    assert_redirected_to login_path(:referer => diary_new_path)
+    assert_redirected_to login_path(:referer => new_diary_entry_path)
     assert_match(/Confirmed your account/, flash[:notice])
   end
 
@@ -488,7 +488,7 @@ class UsersControllerTest < ActionController::TestCase
 
   def test_confirm_already_confirmed
     user = create(:user)
-    confirm_string = user.tokens.create(:referer => diary_new_path).token
+    confirm_string = user.tokens.create(:referer => new_diary_entry_path).token
 
     @request.cookies["_osm_session"] = user.display_name
     post :confirm, :params => { :display_name => user.display_name, :confirm_string => confirm_string }

test/integration/user_diaries_test.rb

@@ -29,7 +29,7 @@ class UserDiariesTest < ActionDispatch::IntegrationTest
     follow_redirect!
 
     assert_response :success
-    assert_template "diary_entries/edit"
+    assert_template "diary_entries/new"
     # print @response.body
     # print @html_document.to_yaml
 
@@ -42,7 +42,7 @@ class UserDiariesTest < ActionDispatch::IntegrationTest
       assert_select "h1", "New Diary Entry"
     end
     assert_select "div#content" do
-      assert_select "form[action='/diary/new']" do
+      assert_select "form[action='/diary']" do
         assert_select "input[id=diary_entry_title]"
       end
     end