From: Andy Allan Date: Wed, 28 Aug 2024 17:52:10 +0000 (+0100) Subject: Merge pull request #5124 from tomhughes/turbo-issue-search X-Git-Tag: live~733 X-Git-Url: https://git.openstreetmap.org/rails.git/commitdiff_plain/7fc83c880ab8ac6e3f6a8778ee7bc65a066670e3?hp=9416317e6428e8b6a801edba8bbbc27ee27dc7cc Merge pull request #5124 from tomhughes/turbo-issue-search Use turbo to update issue search results --- diff --git a/app/controllers/changeset_comments_controller.rb b/app/controllers/changeset_comments_controller.rb deleted file mode 100644 index 637ac7be6..000000000 --- a/app/controllers/changeset_comments_controller.rb +++ /dev/null @@ -1,50 +0,0 @@ -class ChangesetCommentsController < ApplicationController - before_action :authorize_web - before_action :set_locale - - authorize_resource - - before_action -> { check_database_readable(:need_api => true) } - around_action :web_timeout - - ## - # Get a feed of recent changeset comments - def index - if params[:id] - # Extract the arguments - id = params[:id].to_i - - # Find the changeset - changeset = Changeset.find(id) - - # Return comments for this changeset only - @comments = changeset.comments.includes(:author, :changeset).reverse_order.limit(comments_limit) - else - # Return comments - @comments = ChangesetComment.includes(:author, :changeset).where(:visible => true).order("created_at DESC").limit(comments_limit).preload(:changeset) - end - - # Render the result - respond_to do |format| - format.rss - end - rescue OSM::APIBadUserInput - head :bad_request - end - - private - - ## - # Get the maximum number of comments to return - def comments_limit - if params[:limit] - if params[:limit].to_i.positive? && params[:limit].to_i <= 10000 - params[:limit].to_i - else - raise OSM::APIBadUserInput, "Comments limit must be between 1 and 10000" - end - else - 100 - end - end -end diff --git a/app/controllers/feeds/changeset_comments_controller.rb b/app/controllers/feeds/changeset_comments_controller.rb new file mode 100644 index 000000000..c6b355260 --- /dev/null +++ b/app/controllers/feeds/changeset_comments_controller.rb @@ -0,0 +1,52 @@ +module Feeds + class ChangesetCommentsController < ApplicationController + before_action :authorize_web + before_action :set_locale + + authorize_resource + + before_action -> { check_database_readable(:need_api => true) } + around_action :web_timeout + + ## + # Get a feed of recent changeset comments + def index + if params[:changeset_id] + # Extract the arguments + changeset_id = params[:changeset_id].to_i + + # Find the changeset + changeset = Changeset.find(changeset_id) + + # Return comments for this changeset only + @comments = changeset.comments.includes(:author, :changeset).reverse_order.limit(comments_limit) + else + # Return comments + @comments = ChangesetComment.includes(:author, :changeset).where(:visible => true).order("created_at DESC").limit(comments_limit).preload(:changeset) + end + + # Render the result + respond_to do |format| + format.rss + end + rescue OSM::APIBadUserInput + head :bad_request + end + + private + + ## + # Get the maximum number of comments to return + def comments_limit + if params[:limit] + if params[:limit].to_i.positive? && params[:limit].to_i <= 10000 + params[:limit].to_i + else + raise OSM::APIBadUserInput, "Comments limit must be between 1 and 10000" + end + else + 100 + end + end + end +end diff --git a/app/controllers/messages_controller.rb b/app/controllers/messages_controller.rb index 658c43483..7d86796b1 100644 --- a/app/controllers/messages_controller.rb +++ b/app/controllers/messages_controller.rb @@ -88,6 +88,16 @@ class MessagesController < ApplicationController @title = @message.title + render :action => "new" + elsif message.sender == current_user + @message = Message.new( + :recipient => message.recipient, + :title => "Re: #{message.title.sub(/^Re:\s*/, '')}", + :body => "On #{message.sent_on} #{message.sender.display_name} wrote:\n\n#{message.body.gsub(/^/, '> ')}" + ) + + @title = @message.title + render :action => "new" else flash[:notice] = t ".wrong_user", :user => current_user.display_name diff --git a/app/views/diary_entries/_diary_comment.html.erb b/app/views/diary_entries/_diary_comment.html.erb index dbf8a439e..b3e406816 100644 --- a/app/views/diary_entries/_diary_comment.html.erb +++ b/app/views/diary_entries/_diary_comment.html.erb @@ -13,9 +13,9 @@ <% if can? :hide, DiaryComment %> <% if diary_comment.visible? %> - <%= link_to t(".hide_link"), hide_diary_comment_path(diary_comment.diary_entry.user, diary_comment.diary_entry, diary_comment), :method => :post, :data => { :confirm => t(".confirm") } %> + <%= link_to t(".hide_link"), hide_diary_comment_path(diary_comment), :method => :post, :data => { :confirm => t(".confirm") } %> <% else %> - <%= link_to t(".unhide_link"), unhide_diary_comment_path(diary_comment.diary_entry.user, diary_comment.diary_entry, diary_comment), :method => :post, :data => { :confirm => t(".confirm") } %> + <%= link_to t(".unhide_link"), unhide_diary_comment_path(diary_comment), :method => :post, :data => { :confirm => t(".confirm") } %> <% end %> <% end %> diff --git a/app/views/changeset_comments/_comment.html.erb b/app/views/feeds/changeset_comments/_comment.html.erb similarity index 100% rename from app/views/changeset_comments/_comment.html.erb rename to app/views/feeds/changeset_comments/_comment.html.erb diff --git a/app/views/changeset_comments/_comment.rss.builder b/app/views/feeds/changeset_comments/_comment.rss.builder similarity index 100% rename from app/views/changeset_comments/_comment.rss.builder rename to app/views/feeds/changeset_comments/_comment.rss.builder diff --git a/app/views/changeset_comments/index.rss.builder b/app/views/feeds/changeset_comments/index.rss.builder similarity index 79% rename from app/views/changeset_comments/index.rss.builder rename to app/views/feeds/changeset_comments/index.rss.builder index f055c2014..c86b938f7 100644 --- a/app/views/changeset_comments/index.rss.builder +++ b/app/views/feeds/changeset_comments/index.rss.builder @@ -6,7 +6,7 @@ xml.rss("version" => "2.0", else xml.title t(".title_all") end - xml.link url_for(:controller => "site", :action => "index", :only_path => false) + xml.link root_url xml << render(:partial => "comment", :collection => @comments) end diff --git a/app/views/changeset_comments/timeout.atom.builder b/app/views/feeds/changeset_comments/timeout.atom.builder similarity index 100% rename from app/views/changeset_comments/timeout.atom.builder rename to app/views/feeds/changeset_comments/timeout.atom.builder diff --git a/app/views/changeset_comments/timeout.html.erb b/app/views/feeds/changeset_comments/timeout.html.erb similarity index 100% rename from app/views/changeset_comments/timeout.html.erb rename to app/views/feeds/changeset_comments/timeout.html.erb diff --git a/app/views/messages/show.html.erb b/app/views/messages/show.html.erb index 8fe508469..99d6d0435 100644 --- a/app/views/messages/show.html.erb +++ b/app/views/messages/show.html.erb @@ -18,8 +18,8 @@
<%= @message.body.to_html %>
+ <%= link_to t(".reply_button"), message_reply_path(@message), :class => "btn btn-primary" %> <% if current_user == @message.recipient %> - <%= link_to t(".reply_button"), message_reply_path(@message), :class => "btn btn-primary" %> <%= link_to t(".unread_button"), message_mark_path(@message, :mark => "unread"), :method => "post", :class => "btn btn-primary" %> <%= link_to t(".destroy_button"), message_path(@message), :method => "delete", :class => "btn btn-danger" %> <% else %> diff --git a/config/locales/en.yml b/config/locales/en.yml index 1ad0d2ed8..f00702220 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -246,6 +246,18 @@ en: entry: comment: Comment full: Full note + feeds: + changeset_comments: + comment: + comment: "New comment on changeset #%{changeset_id} by %{author}" + commented_at_by_html: "Updated %{when} by %{user}" + comments: + comment: "New comment on changeset #%{changeset_id} by %{author}" + index: + title_all: OpenStreetMap changeset discussion + title_particular: "OpenStreetMap changeset #%{changeset_id} discussion" + timeout: + sorry: "Sorry, the list of changeset comments you requested took too long to retrieve." account: deletions: show: @@ -490,17 +502,6 @@ en: relations_paginated: "Relations (%{x}-%{y} of %{count})" timeout: sorry: "Sorry, the list of changesets you requested took too long to retrieve." - changeset_comments: - comment: - comment: "New comment on changeset #%{changeset_id} by %{author}" - commented_at_by_html: "Updated %{when} by %{user}" - comments: - comment: "New comment on changeset #%{changeset_id} by %{author}" - index: - title_all: OpenStreetMap changeset discussion - title_particular: "OpenStreetMap changeset #%{changeset_id} discussion" - timeout: - sorry: "Sorry, the list of changeset comments you requested took too long to retrieve." dashboards: contact: km away: "%{count}km away" diff --git a/config/routes.rb b/config/routes.rb index b0e23301e..817202486 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -125,8 +125,11 @@ OpenStreetMap::Application.routes.draw do resources :old_relations, :path => "/relation/:id/history", :id => /\d+/, :version => /\d+/, :param => :version, :only => :show resources :changesets, :path => "changeset", :id => /\d+/, :only => :show do match :subscribe, :unsubscribe, :on => :member, :via => [:get, :post] + + namespace :feeds, :path => "" do + resources :changeset_comments, :path => "comments/feed", :only => :index, :defaults => { :format => "rss" } + end end - get "/changeset/:id/comments/feed" => "changeset_comments#index", :as => :changeset_comments_feed, :id => /\d*/, :defaults => { :format => "rss" } resources :notes, :path => "note", :id => /\d+/, :only => [:show, :new] get "/user/:display_name/history" => "changesets#index" @@ -164,7 +167,9 @@ OpenStreetMap::Application.routes.draw do get "/communities" => "site#communities" get "/history" => "changesets#index" get "/history/feed" => "changesets#feed", :defaults => { :format => :atom } - get "/history/comments/feed" => "changeset_comments#index", :as => :changesets_comments_feed, :defaults => { :format => "rss" } + namespace :feeds, :path => "" do + resources :changeset_comments, :path => "/history/comments/feed", :only => :index, :defaults => { :format => "rss" } + end get "/export" => "site#export" get "/login" => "sessions#new" post "/login" => "sessions#create" @@ -255,8 +260,8 @@ OpenStreetMap::Application.routes.draw do match "/user/:display_name/diary/:id/subscribe" => "diary_entries#subscribe", :via => [:get, :post], :as => :diary_entry_subscribe, :id => /\d+/ match "/user/:display_name/diary/:id/unsubscribe" => "diary_entries#unsubscribe", :via => [:get, :post], :as => :diary_entry_unsubscribe, :id => /\d+/ post "/user/:display_name/diary/:id/comments" => "diary_comments#create", :id => /\d+/, :as => :comment_diary_entry - post "/user/:display_name/diary/:id/comments/:comment/hide" => "diary_comments#hide", :id => /\d+/, :comment => /\d+/, :as => :hide_diary_comment - post "/user/:display_name/diary/:id/comments/:comment/unhide" => "diary_comments#unhide", :id => /\d+/, :comment => /\d+/, :as => :unhide_diary_comment + post "/diary_comments/:comment/hide" => "diary_comments#hide", :comment => /\d+/, :as => :hide_diary_comment + post "/diary_comments/:comment/unhide" => "diary_comments#unhide", :comment => /\d+/, :as => :unhide_diary_comment # user pages resources :users, :path => "user", :param => :display_name, :only => [:show, :destroy] diff --git a/lib/tasks/cleanup.rake b/lib/tasks/cleanup.rake new file mode 100644 index 000000000..8c95a1783 --- /dev/null +++ b/lib/tasks/cleanup.rake @@ -0,0 +1,10 @@ +namespace :db do + desc "Expire old tokens" + task :expire_tokens => :environment do + OauthNonce.where("timestamp < EXTRACT(EPOCH FROM NOW() - INTERVAL '1 day')").delete_all + OauthToken.where("invalidated_at < NOW() - INTERVAL '28 days'").delete_all + RequestToken.where("authorized_at IS NULL AND created_at < NOW() - INTERVAL '28 days'").delete_all + Doorkeeper::AccessGrant.where("revoked_at < NOW() - INTERVAL '28 days' OR (created_at + expires_in * INTERVAL '1 second') < NOW() - INTERVAL '28 days'").delete_all + Doorkeeper::AccessToken.where("revoked_at < NOW() - INTERVAL '28 days' OR (created_at + expires_in * INTERVAL '1 second') < NOW() - INTERVAL '28 days'").delete_all + end +end diff --git a/public/robots.txt b/public/robots.txt index 058db57c6..2edd47453 100644 --- a/public/robots.txt +++ b/public/robots.txt @@ -1,3 +1,9 @@ +# OpenStreetMap's data is available for free in bulk from https://planet.openstreetmap.org +# For regional extracts and documentation, see https://wiki.openstreetmap.org/wiki/Planet.osm +# We encourage you to use these instead of scraping our site. +# Scraping puts a high load on our donated resources and will lead to your IP being blocked. +# Please respect our resources, and help us keep the service free and accessible for everyone. + User-agent: * Disallow: /user/*/traces/ Allow: /user/ diff --git a/script/cleanup b/script/cleanup deleted file mode 100755 index e829be176..000000000 --- a/script/cleanup +++ /dev/null @@ -1,12 +0,0 @@ -#!/usr/bin/env ruby - -require File.join(File.dirname(__FILE__), "..", "config", "environment") - -OauthNonce.where("timestamp < EXTRACT(EPOCH FROM NOW() - INTERVAL '1 day')").delete_all -OauthToken.where("invalidated_at < NOW() - INTERVAL '28 days'").delete_all -RequestToken.where("authorized_at IS NULL AND created_at < NOW() - INTERVAL '28 days'").delete_all - -Doorkeeper::AccessGrant.where("revoked_at < NOW() - INTERVAL '28 days' OR (created_at + expires_in * INTERVAL '1 second') < NOW() - INTERVAL '28 days'").delete_all -Doorkeeper::AccessToken.where("revoked_at < NOW() - INTERVAL '28 days' OR (created_at + expires_in * INTERVAL '1 second') < NOW() - INTERVAL '28 days'").delete_all - -exit 0 diff --git a/test/controllers/changeset_comments_controller_test.rb b/test/controllers/changeset_comments_controller_test.rb deleted file mode 100644 index b03640eab..000000000 --- a/test/controllers/changeset_comments_controller_test.rb +++ /dev/null @@ -1,70 +0,0 @@ -require "test_helper" - -class ChangesetCommentsControllerTest < ActionDispatch::IntegrationTest - ## - # test all routes which lead to this controller - def test_routes - assert_routing( - { :path => "/changeset/1/comments/feed", :method => :get }, - { :controller => "changeset_comments", :action => "index", :id => "1", :format => "rss" } - ) - assert_routing( - { :path => "/history/comments/feed", :method => :get }, - { :controller => "changeset_comments", :action => "index", :format => "rss" } - ) - end - - ## - # test comments feed - def test_feed - changeset = create(:changeset, :closed) - create_list(:changeset_comment, 3, :changeset => changeset) - - get changesets_comments_feed_path(:format => "rss") - assert_response :success - assert_equal "application/rss+xml", @response.media_type - assert_select "rss", :count => 1 do - assert_select "channel", :count => 1 do - assert_select "item", :count => 3 - end - end - - get changesets_comments_feed_path(:format => "rss", :limit => 2) - assert_response :success - assert_equal "application/rss+xml", @response.media_type - assert_select "rss", :count => 1 do - assert_select "channel", :count => 1 do - assert_select "item", :count => 2 - end - end - - get changeset_comments_feed_path(:id => changeset.id, :format => "rss") - assert_response :success - assert_equal "application/rss+xml", @response.media_type - last_comment_id = -1 - assert_select "rss", :count => 1 do - assert_select "channel", :count => 1 do - assert_select "item", :count => 3 do |items| - items.each do |item| - assert_select item, "link", :count => 1 do |link| - match = assert_match(/^#{changeset_url changeset}#c(\d+)$/, link.text) - comment_id = match[1].to_i - assert_operator comment_id, "<", last_comment_id if last_comment_id != -1 - last_comment_id = comment_id - end - end - end - end - end - end - - ## - # test comments feed - def test_feed_bad_limit - get changesets_comments_feed_path(:format => "rss", :limit => 0) - assert_response :bad_request - - get changesets_comments_feed_path(:format => "rss", :limit => 100001) - assert_response :bad_request - end -end diff --git a/test/controllers/diary_comments_controller_test.rb b/test/controllers/diary_comments_controller_test.rb index a06565aa1..65a71a9b5 100644 --- a/test/controllers/diary_comments_controller_test.rb +++ b/test/controllers/diary_comments_controller_test.rb @@ -17,12 +17,12 @@ class DiaryCommentsControllerTest < ActionDispatch::IntegrationTest { :controller => "diary_comments", :action => "create", :display_name => "username", :id => "1" } ) assert_routing( - { :path => "/user/username/diary/1/comments/2/hide", :method => :post }, - { :controller => "diary_comments", :action => "hide", :display_name => "username", :id => "1", :comment => "2" } + { :path => "/diary_comments/2/hide", :method => :post }, + { :controller => "diary_comments", :action => "hide", :comment => "2" } ) assert_routing( - { :path => "/user/username/diary/1/comments/2/unhide", :method => :post }, - { :controller => "diary_comments", :action => "unhide", :display_name => "username", :id => "1", :comment => "2" } + { :path => "/diary_comments/2/unhide", :method => :post }, + { :controller => "diary_comments", :action => "unhide", :comment => "2" } ) get "/user/username/diary/comments/1" @@ -186,19 +186,19 @@ class DiaryCommentsControllerTest < ActionDispatch::IntegrationTest diary_comment = create(:diary_comment, :diary_entry => diary_entry) # Try without logging in - post hide_diary_comment_path(user, diary_entry, diary_comment) + post hide_diary_comment_path(diary_comment) assert_response :forbidden assert DiaryComment.find(diary_comment.id).visible # Now try as a normal user session_for(user) - post hide_diary_comment_path(user, diary_entry, diary_comment) + post hide_diary_comment_path(diary_comment) assert_redirected_to :controller => :errors, :action => :forbidden assert DiaryComment.find(diary_comment.id).visible # Try as a moderator session_for(create(:moderator_user)) - post hide_diary_comment_path(user, diary_entry, diary_comment) + post hide_diary_comment_path(diary_comment) assert_redirected_to diary_entry_path(user, diary_entry) assert_not DiaryComment.find(diary_comment.id).visible @@ -207,7 +207,7 @@ class DiaryCommentsControllerTest < ActionDispatch::IntegrationTest # Finally try as an administrator session_for(create(:administrator_user)) - post hide_diary_comment_path(user, diary_entry, diary_comment) + post hide_diary_comment_path(diary_comment) assert_redirected_to diary_entry_path(user, diary_entry) assert_not DiaryComment.find(diary_comment.id).visible end @@ -218,19 +218,19 @@ class DiaryCommentsControllerTest < ActionDispatch::IntegrationTest diary_comment = create(:diary_comment, :diary_entry => diary_entry, :visible => false) # Try without logging in - post unhide_diary_comment_path(user, diary_entry, diary_comment) + post unhide_diary_comment_path(diary_comment) assert_response :forbidden assert_not DiaryComment.find(diary_comment.id).visible # Now try as a normal user session_for(user) - post unhide_diary_comment_path(user, diary_entry, diary_comment) + post unhide_diary_comment_path(diary_comment) assert_redirected_to :controller => :errors, :action => :forbidden assert_not DiaryComment.find(diary_comment.id).visible # Now try as a moderator session_for(create(:moderator_user)) - post unhide_diary_comment_path(user, diary_entry, diary_comment) + post unhide_diary_comment_path(diary_comment) assert_redirected_to diary_entry_path(user, diary_entry) assert DiaryComment.find(diary_comment.id).visible @@ -239,7 +239,7 @@ class DiaryCommentsControllerTest < ActionDispatch::IntegrationTest # Finally try as an administrator session_for(create(:administrator_user)) - post unhide_diary_comment_path(user, diary_entry, diary_comment) + post unhide_diary_comment_path(diary_comment) assert_redirected_to diary_entry_path(user, diary_entry) assert DiaryComment.find(diary_comment.id).visible end diff --git a/test/controllers/feeds/changeset_comments_controller_test.rb b/test/controllers/feeds/changeset_comments_controller_test.rb new file mode 100644 index 000000000..a4149b0d5 --- /dev/null +++ b/test/controllers/feeds/changeset_comments_controller_test.rb @@ -0,0 +1,72 @@ +require "test_helper" + +module Feeds + class ChangesetCommentsControllerTest < ActionDispatch::IntegrationTest + ## + # test all routes which lead to this controller + def test_routes + assert_routing( + { :path => "/changeset/1/comments/feed", :method => :get }, + { :controller => "feeds/changeset_comments", :action => "index", :changeset_id => "1", :format => "rss" } + ) + assert_routing( + { :path => "/history/comments/feed", :method => :get }, + { :controller => "feeds/changeset_comments", :action => "index", :format => "rss" } + ) + end + + ## + # test comments feed + def test_feed + changeset = create(:changeset, :closed) + create_list(:changeset_comment, 3, :changeset => changeset) + + get feeds_changeset_comments_path(:format => "rss") + assert_response :success + assert_equal "application/rss+xml", @response.media_type + assert_select "rss", :count => 1 do + assert_select "channel", :count => 1 do + assert_select "item", :count => 3 + end + end + + get feeds_changeset_comments_path(:format => "rss", :limit => 2) + assert_response :success + assert_equal "application/rss+xml", @response.media_type + assert_select "rss", :count => 1 do + assert_select "channel", :count => 1 do + assert_select "item", :count => 2 + end + end + + get changeset_feeds_changeset_comments_path(changeset, :format => "rss") + assert_response :success + assert_equal "application/rss+xml", @response.media_type + last_comment_id = -1 + assert_select "rss", :count => 1 do + assert_select "channel", :count => 1 do + assert_select "item", :count => 3 do |items| + items.each do |item| + assert_select item, "link", :count => 1 do |link| + match = assert_match(/^#{changeset_url changeset}#c(\d+)$/, link.text) + comment_id = match[1].to_i + assert_operator comment_id, "<", last_comment_id if last_comment_id != -1 + last_comment_id = comment_id + end + end + end + end + end + end + + ## + # test comments feed + def test_feed_bad_limit + get feeds_changeset_comments_path(:format => "rss", :limit => 0) + assert_response :bad_request + + get feeds_changeset_comments_path(:format => "rss", :limit => 100001) + assert_response :bad_request + end + end +end diff --git a/test/controllers/messages_controller_test.rb b/test/controllers/messages_controller_test.rb index b39aed77b..52a856beb 100644 --- a/test/controllers/messages_controller_test.rb +++ b/test/controllers/messages_controller_test.rb @@ -263,6 +263,21 @@ class MessagesControllerTest < ActionDispatch::IntegrationTest end assert Message.find(message.id).message_read + # Login as the sending user + session_for(user) + + # Check that the message reply page loads + get message_reply_path(message) + assert_response :success + assert_template "new" + assert_select "title", "Re: #{message.title} | OpenStreetMap" + assert_select "form[action='/messages']", :count => 1 do + assert_select "input[type='hidden'][name='display_name'][value='#{recipient_user.display_name}']" + assert_select "input#message_title[value='Re: #{message.title}']", :count => 1 + assert_select "textarea#message_body", :count => 1 + assert_select "input[type='submit'][value='Send']", :count => 1 + end + # Asking to reply to a message with a bogus ID should fail get message_reply_path(99999) assert_response :not_found @@ -292,21 +307,23 @@ class MessagesControllerTest < ActionDispatch::IntegrationTest # Login as the message sender session_for(user) - # Check that the message sender can read the message + # Check that the message sender can read the message and that Reply button is available get message_path(message) assert_response :success assert_template "show" assert_select "a[href='#{user_path recipient_user}']", :text => recipient_user.display_name + assert_select "a.btn.btn-primary", :text => "Reply" assert_not Message.find(message.id).message_read # Login as the message recipient session_for(recipient_user) - # Check that the message recipient can read the message + # Check that the message recipient can read the message and that Reply button is available get message_path(message) assert_response :success assert_template "show" assert_select "a[href='#{user_path user}']", :text => user.display_name + assert_select "a.btn.btn-primary", :text => "Reply" assert Message.find(message.id).message_read # Asking to read a message with a bogus ID should fail