Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds optional use of notes records #5511

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/assets/javascripts/index/layers/notes.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ OSM.initializeNotesLayer = function (map) {
} else {
marker = L.marker(feature.geometry.coordinates.reverse(), {
icon: noteIcons[feature.properties.status],
title: feature.properties.comments[0].text,
title: feature.properties.description,
opacity: 0.8,
interactive: true
});
Expand Down
12 changes: 12 additions & 0 deletions app/helpers/note_helper.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,18 @@
module NoteHelper
include ActionView::Helpers::TranslationHelper

def note_description(note)
if note.nil?
""
elsif note.author_deleted?
RichText.new("text", t("notes.show.description_when_author_is_deleted"))
elsif note.user_ip.nil? && note.user_id.nil?
note.all_comments.first.body
else
RichText.new("text", note.description)
end
end

def note_event(event, at, by)
if by.nil?
t("notes.show.event_#{event}_by_anonymous_html",
Expand Down
26 changes: 11 additions & 15 deletions app/models/note.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@
class Note < ApplicationRecord
include GeoRecord

belongs_to :author, :class_name => "User", :foreign_key => "user_id", :optional => true

has_many :comments, -> { left_joins(:author).where(:visible => true, :users => { :status => [nil, "active", "confirmed"] }).order(:created_at) }, :class_name => "NoteComment", :foreign_key => :note_id
has_many :all_comments, -> { left_joins(:author).order(:created_at) }, :class_name => "NoteComment", :foreign_key => :note_id, :inverse_of => :note
has_many :subscriptions, :class_name => "NoteSubscription"
Expand Down Expand Up @@ -89,24 +91,18 @@ def freshly_closed_until
closed_at + DEFAULT_FRESHLY_CLOSED_LIMIT
end

# Return the note's description, derived from the first comment
def description
comments.first.body
def author_deleted?
!author.nil? && author.status == "deleted"
end

# Return the note's author object, derived from the first comment
# Return the note's author object, unless record is unavailable and
# it will be derived from the first comment
def author
comments.first.author
end

# Return the note's author ID, derived from the first comment
def author_id
comments.first.author_id
end

# Return the note's author IP address, derived from the first comment
def author_ip
comments.first.author_ip
if user_ip.nil? && user_id.nil?
all_comments.first.author
else
self[:author]
end
end

private
Expand Down
1 change: 1 addition & 0 deletions app/views/api/notes/_note.json.jbuilder
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ end

json.properties do
json.id note.id
json.description note_description(note)
json.url api_note_url(note, :format => params[:format])

if note.closed?
Expand Down
2 changes: 1 addition & 1 deletion app/views/api/notes/_note.rss.builder
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ xml.item do
xml.guid api_note_url(note)
xml.description render(:partial => "description", :object => note, :formats => [:html])

xml.dc :creator, note.author.display_name if note.author
xml.dc :creator, note.author.display_name unless note.author.nil? || note.author_deleted?

xml.pubDate note.created_at.to_fs(:rfc822)
xml.geo :lat, note.lat
Expand Down
2 changes: 1 addition & 1 deletion app/views/api/notes/_note.xml.builder
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
xml.note("lon" => note.lon, "lat" => note.lat) do
xml.note("lon" => note.lon, "lat" => note.lat, "description" => note_description(note)) do
xml.id note.id
xml.url api_note_url(note, :format => params[:format])

Expand Down
2 changes: 1 addition & 1 deletion app/views/notes/index.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@
</td>
<td><%= link_to note.id, note %></td>
<td><%= note_author(note.author) %></td>
<td><%= note.description.to_html %></td>
<td><%= note_description(note).to_html %></td>
<td><%= friendly_date_ago(note.created_at) %></td>
<td><%= friendly_date_ago(note.updated_at) %></td>
</tr>
Expand Down
6 changes: 3 additions & 3 deletions app/views/notes/show.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
<div>
<h4><%= t(".description") %></h4>
<div class="overflow-hidden ms-2">
<%= h(@note.description.to_html) %>
<%= h(note_description(@note).to_html) %>
</div>

<div class="details" data-coordinates="<%= @note.lat %>,<%= @note.lon %>" data-status="<%= @note.status %>">
Expand Down Expand Up @@ -52,10 +52,10 @@
<% end %>
</div>

<% if @note_comments.length > 1 %>
<% if @note_comments.length > (@note.author_deleted? ? 0 : 1) %>
<div class='note-comments'>
<ul class="list-unstyled">
<% @note_comments.drop(1).each do |comment| %>
<% @note_comments.drop(@note.author_deleted? ? 0 : 1).each do |comment| %>
<li id="c<%= comment.id %>">
<small class='text-body-secondary'><%= note_event(comment.event, comment.created_at, comment.author) %></small>
<div class="mx-2">
Expand Down
1 change: 1 addition & 0 deletions config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3063,6 +3063,7 @@ en:
open_title: "Unresolved note #%{note_name}"
closed_title: "Resolved note #%{note_name}"
hidden_title: "Hidden note #%{note_name}"
description_when_author_is_deleted: "deleted"
event_opened_by_html: "Created by %{user} %{time_ago}"
event_opened_by_anonymous_html: "Created by anonymous %{time_ago}"
event_commented_by_html: "Comment from %{user} %{time_ago}"
Expand Down
26 changes: 0 additions & 26 deletions test/models/note_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -47,15 +47,6 @@ def test_closed?
assert_not_predicate create(:note, :status => "open", :closed_at => nil), :closed?
end

def test_description
comment = create(:note_comment)
assert_equal comment.body, comment.note.description

user = create(:user)
comment = create(:note_comment, :author => user)
assert_equal comment.body, comment.note.description
end

def test_author
comment = create(:note_comment)
assert_nil comment.note.author
Expand All @@ -65,23 +56,6 @@ def test_author
assert_equal user, comment.note.author
end

def test_author_id
comment = create(:note_comment)
assert_nil comment.note.author_id

user = create(:user)
comment = create(:note_comment, :author => user)
assert_equal user.id, comment.note.author_id
end

def test_author_ip
comment = create(:note_comment)
assert_nil comment.note.author_ip

comment = create(:note_comment, :author_ip => IPAddr.new("192.168.1.1"))
assert_equal IPAddr.new("192.168.1.1"), comment.note.author_ip
end

# Ensure the lat/lon is formatted as a decimal e.g. not 4.0e-05
def test_lat_lon_format
note = build(:note, :latitude => 0.00004 * GeoRecord::SCALE, :longitude => 0.00008 * GeoRecord::SCALE)
Expand Down
1 change: 0 additions & 1 deletion test/system/report_note_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ class ReportNoteTest < ApplicationSystemTestCase
def test_no_link_when_not_logged_in
note = create(:note_with_comments)
visit note_path(note)
assert_content note.description

assert_no_content I18n.t("notes.show.report")
end
Expand Down
Loading