From: Tom Hughes Date: Wed, 29 May 2019 16:41:08 +0000 (+0100) Subject: Merge remote-tracking branch 'upstream/pull/2239' X-Git-Tag: live~3108 X-Git-Url: https://git.openstreetmap.org./rails.git/commitdiff_plain/e49953f141a8a090e96652335882927ce872e375?hp=6ed749e7ebad5eeff04ccf1a8f33c078b9f46031 Merge remote-tracking branch 'upstream/pull/2239' --- diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index f989d393d..d63584223 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -40,7 +40,7 @@ Metrics/AbcSize: # Configuration parameters: CountComments, ExcludedMethods. # ExcludedMethods: refine Metrics/BlockLength: - Max: 262 + Max: 263 # Offense count: 11 # Configuration parameters: CountBlocks. diff --git a/app/abilities/ability.rb b/app/abilities/ability.rb index d2864e452..1106d843e 100644 --- a/app/abilities/ability.rb +++ b/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 diff --git a/app/controllers/diary_entries_controller.rb b/app/controllers/diary_entries_controller.rb index 121f74723..1e113e09f 100644 --- a/app/controllers/diary_entries_controller.rb +++ b/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 + 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 - render :action => "edit" + 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 - 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" + 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 diff --git a/app/views/diary_entries/_form.html.erb b/app/views/diary_entries/_form.html.erb new file mode 100644 index 000000000..0d8f7ef5b --- /dev/null +++ b/app/views/diary_entries/_form.html.erb @@ -0,0 +1,35 @@ +
+
+
+ + <%= f.text_field :title, :class => "richtext_title" %> +
+
+ + <%= richtext_area :diary_entry, :body, :cols => 80, :rows => 20, :format => @diary_entry.body_format %> +
+
+ + <%= f.collection_select :language_code, Language.order(:english_name), :code, :name %> +
+
+
+ + <%= content_tag "div", "", :id => "map", :data => { :lat => @lat, :lon => @lon, :zoom => @zoom } %> +
+
+ + <%= f.text_field :latitude, :size => 20, :id => "latitude" %> +
+
+ + <%= f.text_field :longitude, :size => 20, :id => "longitude" %> +
+
+ <%= t ".use_map_link" -%> +
+
+
+ + <%= f.submit %> +
diff --git a/app/views/diary_entries/edit.html.erb b/app/views/diary_entries/edit.html.erb index 62aed884c..5ea619328 100644 --- a/app/views/diary_entries/edit.html.erb +++ b/app/views/diary_entries/edit.html.erb @@ -8,44 +8,6 @@ <%= error_messages_for "diary_entry" %> -<%= form_for :diary_entry do |f| %> -
-
-
- - <%= f.text_field :title, :class => "richtext_title" %> -
-
- - <%= richtext_area :diary_entry, :body, :cols => 80, :rows => 20, :format => @diary_entry.body_format %> -
-
- - <%= f.collection_select :language_code, Language.order(:english_name), :code, :name %> -
-
-
- - <%= content_tag "div", "", :id => "map", :data => { :lat => @lat, :lon => @lon, :zoom => @zoom } %> -
-
- - <%= f.text_field :latitude, :size => 20, :id => "latitude" %> -
-
- - <%= f.text_field :longitude, :size => 20, :id => "longitude" %> -
-
- <%= t ".use_map_link" -%> -
-
-
- - <% if action_name == 'new' %> - <%= submit_tag t("diary_entries.new.publish_button") %> - <% else %> - <%= submit_tag t(".save_button") %> - <% end %> -
+<%= form_for @diary_entry, :url => diary_entry_path(current_user, @diary_entry), :html => { :method => :put } do |f| %> + <%= render :partial => "form", :locals => { :f => f } %> <% end %> diff --git a/app/views/diary_entries/index.html.erb b/app/views/diary_entries/index.html.erb index 4e5dc994d..4610e5fd7 100644 --- a/app/views/diary_entries/index.html.erb +++ b/app/views/diary_entries/index.html.erb @@ -13,13 +13,13 @@ <% if @user %> <% if @user == current_user %>
-
  • <%= link_to image_tag("new.png", :class => "small_icon", :border => 0) + t(".new"), diary_new_path, :title => t(".new_title") %>
    • +
  • <%= link_to image_tag("new.png", :class => "small_icon", :border => 0) + t(".new"), new_diary_entry_path, :title => t(".new_title") %>
    • <% end %> <% else %> <% if current_user %>
    -
  • <%= link_to image_tag("new.png", :class => "small_icon", :border => 0) + t(".new"), diary_new_path, :title => t(".new_title") %>
    • +
  • <%= link_to image_tag("new.png", :class => "small_icon", :border => 0) + t(".new"), new_diary_entry_path, :title => t(".new_title") %>
    • <% end %> <% end %> diff --git a/app/views/diary_entries/new.html.erb b/app/views/diary_entries/new.html.erb new file mode 100644 index 000000000..dfe69f22a --- /dev/null +++ b/app/views/diary_entries/new.html.erb @@ -0,0 +1,13 @@ +<% content_for :head do %> + <%= javascript_include_tag "diary_entry" %> +<% end %> + +<% content_for :heading do %> +

    <%= @title %>

    +<% end %> + +<%= error_messages_for "diary_entry" %> + +<%= form_for @diary_entry do |f| %> + <%= render :partial => "form", :locals => { :f => f } %> +<% end %> diff --git a/app/views/layouts/_header.html.erb b/app/views/layouts/_header.html.erb index a2a63b406..725000a13 100644 --- a/app/views/layouts/_header.html.erb +++ b/app/views/layouts/_header.html.erb @@ -49,7 +49,7 @@ <% end %>
  • <%= link_to t("layouts.gps_traces"), traces_path %>
    • -
  • <%= link_to t("layouts.user_diaries"), diary_path %>
    • +
  • <%= link_to t("layouts.user_diaries"), diary_entries_path %>
  • <%= link_to t("layouts.copyright"), copyright_path %>
  • <%= link_to t("layouts.help"), help_path %>
  • <%= link_to t("layouts.about"), about_path %>
    • @@ -65,7 +65,7 @@ <% end %>
  • <%= link_to t("layouts.gps_traces"), traces_path %>
    • -
  • <%= link_to t("layouts.user_diaries"), diary_path %>
    • +
  • <%= link_to t("layouts.user_diaries"), diary_entries_path %>
  • <%= link_to t("layouts.copyright"), copyright_path %>
  • <%= link_to t("layouts.help"), help_path %>
  • <%= link_to t("layouts.about"), about_path %>
    • diff --git a/app/views/site/about.html.erb b/app/views/site/about.html.erb index 219e0ab6b..886aaf006 100644 --- a/app/views/site/about.html.erb +++ b/app/views/site/about.html.erb @@ -17,7 +17,7 @@

    <%= t ".community_driven_title" %>

    -

    <%= t ".community_driven_html", :diary_path => diary_path %>

    +

    <%= t ".community_driven_html", :diary_path => diary_entries_path %>

    diff --git a/app/views/users/show.html.erb b/app/views/users/show.html.erb index 5090cbee0..87eb9b45f 100644 --- a/app/views/users/show.html.erb +++ b/app/views/users/show.html.erb @@ -205,7 +205,7 @@ <% else %>
    <%= render :partial => "contact", :collection => friends, :locals => { :type => "friend" } %> @@ -221,7 +221,7 @@ <% else %>
    • <%= link_to t(".nearby_changesets"), nearby_changesets_path %>
      • -
    • <%= link_to t(".nearby_diaries"), nearby_diaries_path %>
      • +
    • <%= link_to t(".nearby_diaries"), nearby_diary_entries_path %>
    <%= render :partial => "contact", :collection => nearby, :locals => { :type => "nearby mapper" } %> diff --git a/config/locales/en.yml b/config/locales/en.yml index 575c361bb..ec6591226 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -5,6 +5,11 @@ en: formats: friendly: "%e %B %Y at %H:%M" blog: "%e %B %Y" + helpers: + submit: + diary_entry: + create: "Publish" + update: "Update" activerecord: errors: messages: @@ -282,7 +287,14 @@ en: diary_entries: new: title: New Diary Entry - publish_button: "Publish" + form: + subject: "Subject:" + body: "Body:" + language: "Language:" + location: "Location:" + latitude: "Latitude:" + longitude: "Longitude:" + use_map_link: "use map" index: title: "Users' diaries" title_friends: "Friends' diaries" @@ -296,15 +308,7 @@ en: older_entries: Older Entries newer_entries: Newer Entries edit: - title: "Edit diary entry" - subject: "Subject:" - body: "Body:" - language: "Language:" - location: "Location:" - latitude: "Latitude:" - longitude: "Longitude:" - use_map_link: "use map" - save_button: "Save" + title: Edit Diary Entry marker_text: Diary entry location show: title: "%{user}'s diary | %{title}" diff --git a/config/routes.rb b/config/routes.rb index 198932583..63ce06167 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -216,9 +216,12 @@ OpenStreetMap::Application.routes.draw do post "/trace/:id/delete" => "traces#delete", :id => /\d+/ # diary pages - match "/diary/new" => "diary_entries#new", :via => [:get, :post] - get "/diary/friends" => "diary_entries#index", :friends => true, :as => "friend_diaries" - get "/diary/nearby" => "diary_entries#index", :nearby => true, :as => "nearby_diaries" + resources :diary_entries, :path => "diary", :only => [:new, :create, :index] do + collection do + get "friends" => "diary_entries#index", :friends => true + get "nearby" => "diary_entries#index", :nearby => true + end + end get "/user/:display_name/diary/rss" => "diary_entries#rss", :defaults => { :format => :rss } get "/diary/:language/rss" => "diary_entries#rss", :defaults => { :format => :rss } get "/diary/rss" => "diary_entries#rss", :defaults => { :format => :rss } @@ -226,10 +229,10 @@ OpenStreetMap::Application.routes.draw do get "/user/:display_name/diary/comments/" => "diary_entries#comments" 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+/ diff --git a/test/controllers/diary_entries_controller_test.rb b/test/controllers/diary_entries_controller_test.rb index 6d7ca304a..71684c65e 100644 --- a/test/controllers/diary_entries_controller_test.rb +++ b/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 } @@ -279,21 +279,21 @@ class DiaryEntriesControllerTest < ActionController::TestCase :params => { :display_name => entry.user.display_name, :id => entry.id }, :session => { :user => entry.user } assert_response :success - assert_select "title", :text => /Edit diary entry/, :count => 1 + assert_select "title", :text => /Edit Diary Entry/, :count => 1 assert_select "div.content-heading", :count => 1 do - assert_select "h1", :text => /Edit diary entry/, :count => 1 + 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 assert_select "input#latitude[name='diary_entry[latitude]']", :count => 1 assert_select "input#longitude[name='diary_entry[longitude]']", :count => 1 - assert_select "input[name=commit][type=submit][value=Save]", :count => 1 + assert_select "input[name=commit][type=submit][value=Update]", :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 diff --git a/test/controllers/users_controller_test.rb b/test/controllers/users_controller_test.rb index df2e7d8f5..5dbd866a9 100644 --- a/test/controllers/users_controller_test.rb +++ b/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 } diff --git a/test/integration/user_diaries_test.rb b/test/integration/user_diaries_test.rb index 026028d5f..e090342c1 100644 --- a/test/integration/user_diaries_test.rb +++ b/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 diff --git a/test/system/diary_entry_test.rb b/test/system/diary_entry_test.rb index 39ccc04b2..6b6a51de5 100644 --- a/test/system/diary_entry_test.rb +++ b/test/system/diary_entry_test.rb @@ -8,7 +8,7 @@ class DiaryEntrySystemTest < ApplicationSystemTestCase test "reply to diary entry should prefill the message subject" do sign_in_as(create(:user)) - visit diary_path + visit diary_entries_path click_on "Reply to this entry"