cst1/openstreetmap-website
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Move browse#note to notes#show

Browse source 
This allows a more resourceful routing approach.
This commit is contained in:
Andy Allan 2023-01-25 21:46:35 +00:00
parent b5046fdcd0
commit 9748ce301c
22 changed files with 175 additions and 138 deletions
Download patch file Download diff file Expand all files Collapse all files

4
app/abilities/ability.rb
View file

@ -5,8 +5,8 @@ class Ability
def initialize(user)
can [:relation, :relation_history, :way, :way_history, :node, :node_history,
:changeset, :note, :query], :browse
can [:new], Note
:changeset, :query], :browse
can [:show, :new], Note
can :search, :direction
can [:index, :permalink, :edit, :help, :fixthemap, :offline, :export, :about, :communities, :preview, :copyright, :key, :id], :site
can [:finish, :embed], :export

14
app/controllers/browse_controller.rb
View file

@ -76,19 +76,5 @@ class BrowseController < ApplicationController
render :action => "not_found", :status => :not_found
end
def note
@type = "note"
if current_user&.moderator?
@note = Note.find(params[:id])
@note_comments = @note.comments.unscope(:where => :visible)
else
@note = Note.visible.find(params[:id])
@note_comments = @note.comments
end
rescue ActiveRecord::RecordNotFound
render :action => "not_found", :status => :not_found
end
def query; end
end

14
app/controllers/notes_controller.rb
View file

@ -33,5 +33,19 @@ class NotesController < ApplicationController
end
end
def show
@type = "note"
if current_user&.moderator?
@note = Note.find(params[:id])
@note_comments = @note.comments.unscope(:where => :visible)
else
@note = Note.visible.find(params[:id])
@note_comments = @note.comments
end
rescue ActiveRecord::RecordNotFound
render :template => "browse/not_found", :status => :not_found
end
def new; end
end

2
app/controllers/site_controller.rb
View file

@ -140,7 +140,7 @@ class SiteController < ApplicationController
elsif params[:relation]
redirect_to relation_path(params[:relation])
elsif params[:note]
redirect_to browse_note_path(params[:note])
redirect_to note_path(params[:note])
elsif params[:query]
redirect_to search_path(:query => params[:query])
end

2
app/helpers/issues_helper.rb
View file

@ -8,7 +8,7 @@ module IssuesHelper
when DiaryComment
diary_entry_url(reportable.diary_entry.user, reportable.diary_entry, :anchor => "comment#{reportable.id}")
when Note
url_for(:controller => :browse, :action => :note, :id => reportable.id)
note_url(reportable)
end
end

4
app/helpers/note_helper.rb
View file

@ -3,11 +3,11 @@ module NoteHelper
def note_event(event, at, by)
if by.nil?
t("browse.note.#{event}_by_anonymous_html",
t("notes.show.#{event}_by_anonymous_html",
:when => friendly_date_ago(at),
:exact_time => l(at))
else
t("browse.note.#{event}_by_html",
t("notes.show.#{event}_by_html",
:when => friendly_date_ago(at),
:exact_time => l(at),
:user => note_author(by))

2
app/mailers/user_mailer.rb
View file

@ -123,7 +123,7 @@ class UserMailer < ApplicationMailer
def note_comment_notification(comment, recipient)
with_recipient_locale recipient do
@noteurl = browse_note_url(comment.note)
@noteurl = note_url(comment.note)
@place = Nominatim.describe_location(comment.note.lat, comment.note.lon, 14, I18n.locale)
@comment = comment.body
@owner = recipient == comment.note.author

4
app/views/api/notes/_note.gpx.builder
View file

@ -1,12 +1,12 @@
xml.wpt("lon" => note.lon, "lat" => note.lat) do
xml.time note.created_at.to_fs(:iso8601)
xml.name t("browse.note.title", :id => note.id)
xml.name t("notes.show.title", :id => note.id)
xml.desc do
xml.cdata! render(:partial => "description", :object => note, :formats => [:html])
end
xml.link("href" => browse_note_url(note, :only_path => false))
xml.link("href" => note_url(note, :only_path => false))
xml.extensions do
xml.id note.id

2
app/views/api/notes/_note.rss.builder
View file

@ -9,7 +9,7 @@ xml.item do
xml.title t("api.notes.rss.opened", :place => location)
end
xml.link browse_note_url(note)
xml.link note_url(note)
xml.guid api_note_url(note)
xml.description render(:partial => "description", :object => note, :formats => [:html])

4
app/views/api/notes/feed.rss.builder
View file

@ -15,8 +15,8 @@ xml.rss("version" => "2.0",
xml.item do
xml.title t("api.notes.rss.#{comment.event}", :place => location)
xml.link url_for(:controller => "/browse", :action => "note", :id => comment.note.id, :anchor => "c#{comment.id}", :only_path => false)
xml.guid url_for(:controller => "/browse", :action => "note", :id => comment.note.id, :anchor => "c#{comment.id}", :only_path => false)
xml.link url_for(:controller => "/notes", :action => "show", :id => comment.note.id, :anchor => "c#{comment.id}", :only_path => false)
xml.guid url_for(:controller => "/notes", :action => "show", :id => comment.note.id, :anchor => "c#{comment.id}", :only_path => false)
xml.description do
xml.cdata! render(:partial => "entry", :object => comment, :formats => [:html])

2
app/views/notes/index.html.erb
View file

@ -29,7 +29,7 @@
<%= image_tag("open_note_marker.png", :alt => "open", :size => "25x40") %>
<% end %>
</td>
<td><%= link_to note.id, browse_note_path(note) %></td>
<td><%= link_to note.id, note %></td>
<td><%= note_author(note.author) %></td>
<td><%= note.comments.first.body.to_html %></td>
<td><%= friendly_date_ago(note.created_at) %></td>

4
app/views/notes/new.html.erb
View file

@ -1,6 +1,6 @@
<% set_title(t("browse.note.new_note")) %>
<% set_title(t(".title")) %>
<%= render "sidebar_header", :title => t("browse.note.new_note") %>
<%= render "sidebar_header", :title => t(".title") %>
<div class="note">
<p class="alert alert-info"><%= t("javascripts.notes.new.intro") %></p>

0
app/views/browse/note.html.erb → app/views/notes/show.html.erb
View file

37
config/locales/en.yml
View file

@ -407,24 +407,6 @@ en:
telephone_link: "Call %{phone_number}"
colour_preview: "Colour %{colour_value} preview"
email_link: "Email %{email}"
note:
title: "Note: %{id}"
new_note: "New Note"
description: "Description"
open_title: "Unresolved note #%{note_name}"
closed_title: "Resolved note #%{note_name}"
hidden_title: "Hidden note #%{note_name}"
opened_by_html: "Created by %{user} <abbr title='%{exact_time}'>%{when}</abbr>"
opened_by_anonymous_html: "Created by anonymous <abbr title='%{exact_time}'>%{when}</abbr>"
commented_by_html: "Comment from %{user} <abbr title='%{exact_time}'>%{when}</abbr>"
commented_by_anonymous_html: "Comment from anonymous <abbr title='%{exact_time}'>%{when}</abbr>"
closed_by_html: "Resolved by %{user} <abbr title='%{exact_time}'>%{when}</abbr>"
closed_by_anonymous_html: "Resolved by anonymous <abbr title='%{exact_time}'>%{when}</abbr>"
reopened_by_html: "Reactivated by %{user} <abbr title='%{exact_time}'>%{when}</abbr>"
reopened_by_anonymous_html: "Reactivated by anonymous <abbr title='%{exact_time}'>%{when}</abbr>"
hidden_by_html: "Hidden by %{user} <abbr title='%{exact_time}'>%{when}</abbr>"
report: report this note
coordinates_html: "%{latitude}, %{longitude}"
query:
title: "Query Features"
introduction: "Click on the map to find nearby features."
@ -2837,6 +2819,25 @@ en:
description: "Description"
created_at: "Created at"
last_changed: "Last changed"
show:
title: "Note: %{id}"
description: "Description"
open_title: "Unresolved note #%{note_name}"
closed_title: "Resolved note #%{note_name}"
hidden_title: "Hidden note #%{note_name}"
opened_by_html: "Created by %{user} <abbr title='%{exact_time}'>%{when}</abbr>"
opened_by_anonymous_html: "Created by anonymous <abbr title='%{exact_time}'>%{when}</abbr>"
commented_by_html: "Comment from %{user} <abbr title='%{exact_time}'>%{when}</abbr>"
commented_by_anonymous_html: "Comment from anonymous <abbr title='%{exact_time}'>%{when}</abbr>"
closed_by_html: "Resolved by %{user} <abbr title='%{exact_time}'>%{when}</abbr>"
closed_by_anonymous_html: "Resolved by anonymous <abbr title='%{exact_time}'>%{when}</abbr>"
reopened_by_html: "Reactivated by %{user} <abbr title='%{exact_time}'>%{when}</abbr>"
reopened_by_anonymous_html: "Reactivated by anonymous <abbr title='%{exact_time}'>%{when}</abbr>"
hidden_by_html: "Hidden by %{user} <abbr title='%{exact_time}'>%{when}</abbr>"
report: report this note
coordinates_html: "%{latitude}, %{longitude}"
new:
title: "New Note"
javascripts:
close: Close
share:

3
config/routes.rb
View file

@ -113,8 +113,7 @@ OpenStreetMap::Application.routes.draw do
get "/relation/:id/history" => "browse#relation_history", :id => /\d+/, :as => :relation_history
get "/changeset/:id" => "browse#changeset", :as => :changeset, :id => /\d+/
get "/changeset/:id/comments/feed" => "changeset_comments#index", :as => :changeset_comments_feed, :id => /\d*/, :defaults => { :format => "rss" }
get "/note/:id" => "browse#note", :id => /\d+/, :as => "browse_note"
resources :notes, :path => "note", :only => [:new]
resources :notes, :path => "note", :only => [:show, :new]
get "/user/:display_name/history" => "changesets#index"
get "/user/:display_name/history/feed" => "changesets#feed", :defaults => { :format => :atom }

2
test/controllers/api/notes_controller_test.rb
View file

@ -492,7 +492,7 @@ module Api
assert_select "rss", :count => 1 do
assert_select "channel", :count => 1 do
assert_select "item", :count => 1 do
assert_select "link", browse_note_url(open_note)
assert_select "link", note_url(open_note)
assert_select "guid", api_note_url(open_note)
assert_select "pubDate", open_note.created_at.to_fs(:rfc822)
assert_select "geo|lat", open_note.lat.to_s

76
test/controllers/browse_controller_test.rb
View file

@ -32,10 +32,6 @@ class BrowseControllerTest < ActionDispatch::IntegrationTest
{ :path => "/changeset/1", :method => :get },
{ :controller => "browse", :action => "changeset", :id => "1" }
)
assert_routing(
{ :path => "/note/1", :method => :get },
{ :controller => "browse", :action => "note", :id => "1" }
)
assert_routing(
{ :path => "/query", :method => :get },
{ :controller => "browse", :action => "query" }
@ -102,78 +98,6 @@ class BrowseControllerTest < ActionDispatch::IntegrationTest
assert_select "div.changeset-comments ul li", :count => 4
end
def test_read_note
open_note = create(:note_with_comments)
browse_check :browse_note_path, open_note.id, "browse/note"
end
def test_read_hidden_note
hidden_note_with_comment = create(:note_with_comments, :status => "hidden")
get browse_note_path(:id => hidden_note_with_comment)
assert_response :not_found
assert_template "browse/not_found"
assert_template :layout => "map"
get browse_note_path(:id => hidden_note_with_comment), :xhr => true
assert_response :not_found
assert_template "browse/not_found"
assert_template :layout => "xhr"
session_for(create(:moderator_user))
browse_check :browse_note_path, hidden_note_with_comment.id, "browse/note"
end
def test_read_note_hidden_comments
note_with_hidden_comment = create(:note_with_comments, :comments_count => 2) do |note|
create(:note_comment, :note => note, :visible => false)
end
browse_check :browse_note_path, note_with_hidden_comment.id, "browse/note"
assert_select "div.note-comments ul li", :count => 1
session_for(create(:moderator_user))
browse_check :browse_note_path, note_with_hidden_comment.id, "browse/note"
assert_select "div.note-comments ul li", :count => 2
end
def test_read_note_hidden_user_comment
hidden_user = create(:user, :deleted)
note_with_hidden_user_comment = create(:note_with_comments, :comments_count => 2) do |note|
create(:note_comment, :note => note, :author => hidden_user)
end
browse_check :browse_note_path, note_with_hidden_user_comment.id, "browse/note"
assert_select "div.note-comments ul li", :count => 1
session_for(create(:moderator_user))
browse_check :browse_note_path, note_with_hidden_user_comment.id, "browse/note"
assert_select "div.note-comments ul li", :count => 1
end
def test_read_closed_note
user = create(:user)
closed_note = create(:note_with_comments, :status => "closed", :closed_at => Time.now.utc, :comments_count => 2) do |note|
create(:note_comment, :event => "closed", :note => note, :author => user)
end
browse_check :browse_note_path, closed_note.id, "browse/note"
assert_select "div.note-comments ul li", :count => 2
assert_select "div.details", /Resolved by #{user.display_name}/
user.soft_destroy!
reset!
browse_check :browse_note_path, closed_note.id, "browse/note"
assert_select "div.note-comments ul li", :count => 1
assert_select "div.details", /Resolved by deleted/
end
##
# Methods to check redaction.
#

115
test/controllers/notes_controller_test.rb
View file

@ -15,7 +15,10 @@ class NotesControllerTest < ActionDispatch::IntegrationTest
{ :path => "/user/username/notes", :method => :get },
{ :controller => "notes", :action => "index", :display_name => "username" }
)
assert_routing(
{ :path => "/note/1", :method => :get },
{ :controller => "notes", :action => "show", :id => "1" }
)
assert_routing(
{ :path => "/note/new", :method => :get },
{ :controller => "notes", :action => "new" }
@ -86,9 +89,119 @@ class NotesControllerTest < ActionDispatch::IntegrationTest
assert_select "h4", :html => "No notes"
end
def test_read_note
open_note = create(:note_with_comments)
browse_check :note_path, open_note.id, "notes/show"
end
def test_read_hidden_note
hidden_note_with_comment = create(:note_with_comments, :status => "hidden")
get note_path(:id => hidden_note_with_comment)
assert_response :not_found
assert_template "browse/not_found"
assert_template :layout => "map"
get note_path(:id => hidden_note_with_comment), :xhr => true
assert_response :not_found
assert_template "browse/not_found"
assert_template :layout => "xhr"
session_for(create(:moderator_user))
browse_check :note_path, hidden_note_with_comment.id, "notes/show"
end
def test_read_note_hidden_comments
note_with_hidden_comment = create(:note_with_comments, :comments_count => 2) do |note|
create(:note_comment, :note => note, :visible => false)
end
browse_check :note_path, note_with_hidden_comment.id, "notes/show"
assert_select "div.note-comments ul li", :count => 1
session_for(create(:moderator_user))
browse_check :note_path, note_with_hidden_comment.id, "notes/show"
assert_select "div.note-comments ul li", :count => 2
end
def test_read_note_hidden_user_comment
hidden_user = create(:user, :deleted)
note_with_hidden_user_comment = create(:note_with_comments, :comments_count => 2) do |note|
create(:note_comment, :note => note, :author => hidden_user)
end
browse_check :note_path, note_with_hidden_user_comment.id, "notes/show"
assert_select "div.note-comments ul li", :count => 1
session_for(create(:moderator_user))
browse_check :note_path, note_with_hidden_user_comment.id, "notes/show"
assert_select "div.note-comments ul li", :count => 1
end
def test_read_closed_note
user = create(:user)
closed_note = create(:note_with_comments, :status => "closed", :closed_at => Time.now.utc, :comments_count => 2) do |note|
create(:note_comment, :event => "closed", :note => note, :author => user)
end
browse_check :note_path, closed_note.id, "notes/show"
assert_select "div.note-comments ul li", :count => 2
assert_select "div.details", /Resolved by #{user.display_name}/
user.soft_destroy!
reset!
browse_check :note_path, closed_note.id, "notes/show"
assert_select "div.note-comments ul li", :count => 1
assert_select "div.details", /Resolved by deleted/
end
def test_new_note
get new_note_path
assert_response :success
assert_template "notes/new"
end
private
# This is a convenience method for most of the above checks
# First we check that when we don't have an id, it will correctly return a 404
# then we check that we get the correct 404 when a non-existant id is passed
# then we check that it will get a successful response, when we do pass an id
def browse_check(path, id, template)
path_method = method(path)
assert_raise ActionController::UrlGenerationError do
get path_method.call
end
# assert_raise ActionController::UrlGenerationError do
# get path_method.call(:id => -10) # we won't have an id that's negative
# end
get path_method.call(:id => 0)
assert_response :not_found
assert_template "browse/not_found"
assert_template :layout => "map"
get path_method.call(:id => 0), :xhr => true
assert_response :not_found
assert_template "browse/not_found"
assert_template :layout => "xhr"
get path_method.call(:id => id)
assert_response :success
assert_template template
assert_template :layout => "map"
get path_method.call(:id => id), :xhr => true
assert_response :success
assert_template template
assert_template :layout => "xhr"
end
end

2
test/controllers/site_controller_test.rb
View file

@ -98,7 +98,7 @@ class SiteControllerTest < ActionDispatch::IntegrationTest
assert_redirected_to :controller => :browse, :action => :relation, :id => 123
get root_path(:note => 123)
assert_redirected_to :controller => :browse, :action => :note, :id => 123
assert_redirected_to :controller => :notes, :action => :show, :id => 123
get root_path(:query => "test")
assert_redirected_to :controller => :geocoder, :action => :search, :query => "test"

6
test/system/index_test.rb
View file

@ -12,7 +12,7 @@ class IndexTest < ApplicationSystemTestCase
test "note included in edit link" do
note = create(:note_with_comments)
visit browse_note_path(note)
visit note_path(note)
assert_selector "#editanchor[href*='?note=#{note.id}#']"
find("#sidebar .btn-close").click
@ -27,8 +27,8 @@ class IndexTest < ApplicationSystemTestCase
visible_note = create(:note, :latitude => position, :longitude => position)
create(:note_comment, :note => visible_note, :body => "this-is-a-visible-note")
visit root_path(:anchor => "map=15/1/1") # view place of hidden note in case it is not rendered during browse_note_path(hidden_note)
visit browse_note_path(hidden_note)
visit root_path(:anchor => "map=15/1/1") # view place of hidden note in case it is not rendered during note_path(hidden_note)
visit note_path(hidden_note)
find(".leaflet-control.control-layers .control-button").click
find("#map-ui .overlay-layers .form-check-label", :text => "Map Notes").click
visible_note_marker = find(".leaflet-marker-icon[title=this-is-a-visible-note]")

12
test/system/report_note_test.rb
View file

@ -3,18 +3,18 @@ require "application_system_test_case"
class ReportNoteTest < ApplicationSystemTestCase
def test_no_link_when_not_logged_in
note = create(:note_with_comments)
visit browse_note_path(note)
visit note_path(note)
assert_content note.comments.first.body
assert_no_content I18n.t("browse.note.report")
assert_no_content I18n.t("notes.show.report")
end
def test_can_report_anonymous_notes
note = create(:note_with_comments)
sign_in_as(create(:user))
visit browse_note_path(note)
visit note_path(note)
click_on I18n.t("browse.note.report")
click_on I18n.t("notes.show.report")
assert_content "Report"
assert_content I18n.t("reports.new.disclaimer.intro")
@ -33,9 +33,9 @@ class ReportNoteTest < ApplicationSystemTestCase
def test_can_report_notes_with_author
note = create(:note_comment, :author => create(:user)).note
sign_in_as(create(:user))
visit browse_note_path(note)
visit note_path(note)
click_on I18n.t("browse.note.report")
click_on I18n.t("notes.show.report")
assert_content "Report"
assert_content I18n.t("reports.new.disclaimer.intro")

2
test/system/report_user_test.rb
View file

@ -3,7 +3,7 @@ require "application_system_test_case"
class ReportUserTest < ApplicationSystemTestCase
def test_no_link_when_not_logged_in
note = create(:note_with_comments)
visit browse_note_path(note)
visit note_path(note)
assert_content note.comments.first.body
assert_no_content I18n.t("users.show.report")