From: Tom Hughes Date: Wed, 3 Oct 2018 17:59:33 +0000 (+0100) Subject: Merge remote-tracking branch 'upstream/pull/2014' X-Git-Tag: live~3850 X-Git-Url: https://git.openstreetmap.org./rails.git/commitdiff_plain/b8a8a88004d25837a10436fdc13128146b32c32b?ds=sidebyside;hp=-c Merge remote-tracking branch 'upstream/pull/2014' --- b8a8a88004d25837a10436fdc13128146b32c32b diff --combined .rubocop.yml index cb301ece1,abf25d3cd..b33f9046c --- a/.rubocop.yml +++ b/.rubocop.yml @@@ -45,7 -45,7 +45,7 @@@ Rails/InverseOf Rails/SkipsModelValidations: Exclude: - 'db/migrate/*.rb' - - 'app/controllers/user_controller.rb' + - 'app/controllers/users_controller.rb' Style/BracesAroundHashParameters: EnforcedStyle: context_dependent @@@ -56,6 -56,10 +56,6 @@@ Style/FormatStringToken Style/IfInsideElse: Enabled: false -Style/GlobalVars: - Exclude: - - 'lib/quad_tile/extconf.rb' - Style/GuardClause: Enabled: false diff --combined .rubocop_todo.yml index d3e51600d,00851ffcc..8fc701cb3 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@@ -14,7 -14,7 +14,7 @@@ Lint/AssignmentInCondition - 'app/controllers/geocoder_controller.rb' - 'app/controllers/notes_controller.rb' - 'app/controllers/traces_controller.rb' - - 'app/controllers/user_controller.rb' + - 'app/controllers/users_controller.rb' - 'app/controllers/user_preferences_controller.rb' - 'app/helpers/application_helper.rb' - 'app/helpers/browse_helper.rb' @@@ -28,7 -28,7 +28,7 @@@ Lint/HandleExceptions: Exclude: - 'app/controllers/amf_controller.rb' - - 'app/controllers/user_controller.rb' + - 'app/controllers/users_controller.rb' # Offense count: 692 Metrics/AbcSize: @@@ -38,7 -38,7 +38,7 @@@ # Configuration parameters: CountComments, ExcludedMethods. # ExcludedMethods: refine Metrics/BlockLength: - Max: 259 + Max: 262 # Offense count: 11 # Configuration parameters: CountBlocks. @@@ -139,7 -139,7 +139,7 @@@ Rails/NotNullColumn # Offense count: 20 Rails/OutputSafety: Exclude: - - 'app/controllers/user_controller.rb' + - 'app/controllers/users_controller.rb' - 'app/helpers/application_helper.rb' - 'app/helpers/changeset_helper.rb' - 'app/helpers/geocoder_helper.rb' diff --combined app/controllers/messages_controller.rb index cf3995b2b,cebf5d95b..13a395da8 --- a/app/controllers/messages_controller.rb +++ b/app/controllers/messages_controller.rb @@@ -54,7 -54,7 +54,7 @@@ class MessagesController < ApplicationC render :action => "new" else flash[:notice] = t ".wrong_user", :user => current_user.display_name - redirect_to :controller => "user", :action => "login", :referer => request.fullpath + redirect_to :controller => "users", :action => "login", :referer => request.fullpath end rescue ActiveRecord::RecordNotFound @title = t "messages.no_such_message.title" @@@ -71,7 -71,7 +71,7 @@@ @message.save else flash[:notice] = t ".wrong_user", :user => current_user.display_name - redirect_to :controller => "user", :action => "login", :referer => request.fullpath + redirect_to :controller => "users", :action => "login", :referer => request.fullpath end rescue ActiveRecord::RecordNotFound @title = t "messages.no_such_message.title" @@@ -110,7 -110,7 +110,7 @@@ # Destroy the message. def destroy - @message = Message.where("to_user_id = ? OR from_user_id = ?", current_user.id, current_user.id).find(params[:message_id]) + @message = Message.where("to_user_id = ? OR from_user_id = ?", current_user.id, current_user.id).find(params[:id]) @message.from_user_visible = false if @message.sender == current_user @message.to_user_visible = false if @message.recipient == current_user if @message.save && !request.xhr? diff --combined app/models/notifier.rb index 01b807607,2ff9c85d7..4b53c66f6 --- a/app/models/notifier.rb +++ b/app/models/notifier.rb @@@ -8,7 -8,7 +8,7 @@@ class Notifier < ActionMailer::Bas def signup_confirm(user, token) with_recipient_locale user do - @url = url_for(:controller => "user", :action => "confirm", + @url = url_for(:controller => "users", :action => "confirm", :display_name => user.display_name, :confirm_string => token.token) @@@ -20,7 -20,7 +20,7 @@@ def email_confirm(user, token) with_recipient_locale user do @address = user.new_email - @url = url_for(:controller => "user", :action => "confirm_email", + @url = url_for(:controller => "users", :action => "confirm_email", :confirm_string => token.token) mail :to => user.new_email, @@@ -30,7 -30,7 +30,7 @@@ def lost_password(user, token) with_recipient_locale user do - @url = url_for(:controller => "user", :action => "reset_password", + @url = url_for(:controller => "users", :action => "reset_password", :token => token.token) mail :to => user.email, @@@ -70,7 -70,7 +70,7 @@@ @text = message.body @title = message.title @readurl = message_url(message) - @replyurl = reply_message_url(message) + @replyurl = message_reply_url(message) @author = @from_user attach_user_avatar(message.sender) @@@ -105,7 -105,7 +105,7 @@@ with_recipient_locale friend.befriendee do @friend = friend @viewurl = user_url(@friend.befriender) - @friendurl = url_for(:controller => "user", :action => "make_friend", + @friendurl = url_for(:controller => "users", :action => "make_friend", :display_name => @friend.befriender.display_name) @author = @friend.befriender.display_name diff --combined config/routes.rb index b2a0f84c3,2750485b9..729e71efa --- a/config/routes.rb +++ b/config/routes.rb @@@ -64,10 -64,10 +64,10 @@@ OpenStreetMap::Application.routes.draw get "relations/search" => "search#search_relations" get "nodes/search" => "search#search_nodes" - get "user/:id" => "user#api_read", :id => /\d+/ - get "user/details" => "user#api_details" - get "user/gpx_files" => "user#api_gpx_files" - get "users" => "user#api_users", :as => :api_users + get "user/:id" => "users#api_read", :id => /\d+/ + get "user/details" => "users#api_details" + get "user/gpx_files" => "users#api_gpx_files" + get "users" => "users#api_users", :as => :api_users get "user/preferences" => "user_preferences#read" get "user/preferences/:preference_key" => "user_preferences#read_one" @@@ -154,33 -154,33 +154,33 @@@ get "/history/feed" => "changeset#feed", :defaults => { :format => :atom } get "/history/comments/feed" => "changeset#comments_feed", :as => :changesets_comments_feed, :defaults => { :format => "rss" } get "/export" => "site#export" - match "/login" => "user#login", :via => [:get, :post] - match "/logout" => "user#logout", :via => [:get, :post] + match "/login" => "users#login", :via => [:get, :post] + match "/logout" => "users#logout", :via => [:get, :post] get "/offline" => "site#offline" get "/key" => "site#key" get "/id" => "site#id" get "/query" => "browse#query" - get "/user/new" => "user#new" - post "/user/new" => "user#create" - get "/user/terms" => "user#terms" - post "/user/save" => "user#save" - get "/user/:display_name/confirm/resend" => "user#confirm_resend" - match "/user/:display_name/confirm" => "user#confirm", :via => [:get, :post] - match "/user/confirm" => "user#confirm", :via => [:get, :post] - match "/user/confirm-email" => "user#confirm_email", :via => [:get, :post] - post "/user/go_public" => "user#go_public" - match "/user/reset-password" => "user#reset_password", :via => [:get, :post] - match "/user/forgot-password" => "user#lost_password", :via => [:get, :post] - get "/user/suspended" => "user#suspended" + get "/user/new" => "users#new" + post "/user/new" => "users#create" + get "/user/terms" => "users#terms" + post "/user/save" => "users#save" + get "/user/:display_name/confirm/resend" => "users#confirm_resend" + match "/user/:display_name/confirm" => "users#confirm", :via => [:get, :post] + match "/user/confirm" => "users#confirm", :via => [:get, :post] + match "/user/confirm-email" => "users#confirm_email", :via => [:get, :post] + post "/user/go_public" => "users#go_public" + match "/user/reset-password" => "users#reset_password", :via => [:get, :post] + match "/user/forgot-password" => "users#lost_password", :via => [:get, :post] + get "/user/suspended" => "users#suspended" get "/index.html", :to => redirect(:path => "/") get "/create-account.html", :to => redirect(:path => "/user/new") get "/forgot-password.html", :to => redirect(:path => "/user/forgot-password") # omniauth - get "/auth/failure" => "user#auth_failure" - match "/auth/:provider/callback" => "user#auth_success", :via => [:get, :post], :as => :auth_success - match "/auth/:provider" => "user#auth", :via => [:get, :post], :as => :auth + get "/auth/failure" => "users#auth_failure" + match "/auth/:provider/callback" => "users#auth_success", :via => [:get, :post], :as => :auth_success + match "/auth/:provider" => "users#auth", :via => [:get, :post], :as => :auth # permalink get "/go/:code" => "site#permalink", :code => /[a-zA-Z0-9_@~]+[=-]*/ @@@ -234,16 -234,16 +234,16 @@@ post "/user/:display_name/diary/:id/unsubscribe" => "diary_entry#unsubscribe", :as => :diary_entry_unsubscribe, :id => /\d+/ # user pages - get "/user/:display_name" => "user#show", :as => "user" - match "/user/:display_name/make_friend" => "user#make_friend", :via => [:get, :post], :as => "make_friend" - match "/user/:display_name/remove_friend" => "user#remove_friend", :via => [:get, :post], :as => "remove_friend" - match "/user/:display_name/account" => "user#account", :via => [:get, :post] - get "/user/:display_name/set_status" => "user#set_status", :as => :set_status_user - get "/user/:display_name/delete" => "user#delete", :as => :delete_user + get "/user/:display_name" => "users#show", :as => "user" + match "/user/:display_name/make_friend" => "users#make_friend", :via => [:get, :post], :as => "make_friend" + match "/user/:display_name/remove_friend" => "users#remove_friend", :via => [:get, :post], :as => "remove_friend" + match "/user/:display_name/account" => "users#account", :via => [:get, :post] + get "/user/:display_name/set_status" => "users#set_status", :as => :set_status_user + get "/user/:display_name/delete" => "users#delete", :as => :delete_user # user lists - match "/users" => "user#index", :via => [:get, :post] - match "/users/:status" => "user#index", :via => [:get, :post] + match "/users" => "users#index", :via => [:get, :post] + match "/users/:status" => "users#index", :via => [:get, :post] # geocoder get "/search" => "geocoder#search" @@@ -262,9 -262,7 +262,9 @@@ get "/export/embed" => "export#embed" # messages - resources :messages, :only => [:create, :show] do + resources :messages, :only => [:create, :show, :destroy] do + post :mark + match :reply, :via => [:get, :post] collection do get :inbox get :outbox @@@ -274,8 -272,9 +274,8 @@@ get "/user/:display_name/outbox", :to => redirect(:path => "/messages/outbox") get "/message/new/:display_name" => "messages#new", :as => "new_message" get "/message/read/:message_id", :to => redirect(:path => "/messages/%{message_id}") - post "/message/mark/:message_id" => "messages#mark", :as => "mark_message" - match "/message/reply/:message_id" => "messages#reply", :via => [:get, :post], :as => "reply_message" - post "/message/delete/:message_id" => "messages#destroy", :as => "destroy_message" + post "/message/mark/:message_id" => "messages#mark" # remove after deployment + match "/message/reply/:message_id" => "messages#reply", :via => [:get, :post] # remove after deployment # oauth admin pages (i.e: for setting up new clients, etc...) scope "/user/:display_name" do diff --combined test/controllers/messages_controller_test.rb index 8536a1b01,8926f7311..0b3a59b39 --- a/test/controllers/messages_controller_test.rb +++ b/test/controllers/messages_controller_test.rb @@@ -25,20 -25,20 +25,20 @@@ class MessagesControllerTest < ActionCo { :controller => "messages", :action => "show", :id => "1" } ) assert_routing( - { :path => "/message/mark/1", :method => :post }, + { :path => "/messages/1/mark", :method => :post }, { :controller => "messages", :action => "mark", :message_id => "1" } ) assert_routing( - { :path => "/message/reply/1", :method => :get }, + { :path => "/messages/1/reply", :method => :get }, { :controller => "messages", :action => "reply", :message_id => "1" } ) assert_routing( - { :path => "/message/reply/1", :method => :post }, + { :path => "/messages/1/reply", :method => :post }, { :controller => "messages", :action => "reply", :message_id => "1" } ) assert_routing( - { :path => "/message/delete/1", :method => :post }, - { :controller => "messages", :action => "destroy", :message_id => "1" } + { :path => "/messages/1", :method => :delete }, + { :controller => "messages", :action => "destroy", :id => "1" } ) end @@@ -195,7 -195,7 +195,7 @@@ # Asking to send a message with a bogus user name should fail get :new, :params => { :display_name => "non_existent_user" } assert_response :not_found - assert_template "user/no_such_user" + assert_template "users/no_such_user" assert_select "h1", "The user non_existent_user does not exist" end @@@ -232,14 -232,14 +232,14 @@@ # Check that the message reply page requires us to login get :reply, :params => { :message_id => unread_message.id } - assert_redirected_to login_path(:referer => reply_message_path(:message_id => unread_message.id)) + assert_redirected_to login_path(:referer => message_reply_path(:message_id => unread_message.id)) # Login as the wrong user session[:user] = other_user.id # Check that we can't reply to somebody else's message get :reply, :params => { :message_id => unread_message.id } - assert_redirected_to login_path(:referer => reply_message_path(:message_id => unread_message.id)) + assert_redirected_to login_path(:referer => message_reply_path(:message_id => unread_message.id)) assert_equal "You are logged in as `#{other_user.display_name}' but the message you have asked to reply to was not sent to that user. Please login as the correct user in order to reply.", flash[:notice] # Login as the right user @@@ -429,14 -429,14 +429,14 @@@ sent_message = create(:message, :unread, :recipient => second_user, :sender => user) # Check that destroying a message requires us to login - post :destroy, :params => { :message_id => read_message.id } + delete :destroy, :params => { :id => read_message.id } assert_response :forbidden # Login as a user with no messages session[:user] = other_user.id # Check that destroying a message we didn't send or receive fails - post :destroy, :params => { :message_id => read_message.id } + delete :destroy, :params => { :id => read_message.id } assert_response :not_found assert_template "no_such_message" @@@ -444,7 -444,7 +444,7 @@@ session[:user] = user.id # Check that the destroy a received message works - post :destroy, :params => { :message_id => read_message.id } + delete :destroy, :params => { :id => read_message.id } assert_redirected_to inbox_messages_path assert_equal "Message deleted", flash[:notice] m = Message.find(read_message.id) @@@ -452,7 -452,7 +452,7 @@@ assert_equal false, m.to_user_visible # Check that the destroying a sent message works - post :destroy, :params => { :message_id => sent_message.id, :referer => outbox_messages_path } + delete :destroy, :params => { :id => sent_message.id, :referer => outbox_messages_path } assert_redirected_to outbox_messages_path assert_equal "Message deleted", flash[:notice] m = Message.find(sent_message.id) @@@ -465,7 -465,7 +465,7 @@@ end # Asking to destroy a message with a bogus ID should fail - post :destroy, :params => { :message_id => 99999 } + delete :destroy, :params => { :id => 99999 } assert_response :not_found assert_template "no_such_message" end