Remove the author_name field from notes
authorTom Hughes <tom@compton.nu>
Sun, 2 Dec 2012 16:29:00 +0000 (16:29 +0000)
committerTom Hughes <tom@compton.nu>
Mon, 3 Dec 2012 14:50:54 +0000 (14:50 +0000)
16 files changed:
app/assets/javascripts/templates/notes/show.jst.ejs
app/controllers/notes_controller.rb
app/helpers/application_helper.rb
app/models/note.rb
app/models/note_comment.rb
app/models/notifier.rb
app/views/notes/_note.json.jsonify
app/views/notes/_note.rss.builder
app/views/notes/_note.xml.builder
app/views/notes/feed.rss.builder
app/views/notes/mine.html.erb
config/locales/en.yml
db/migrate/20121202155309_remove_author_name_from_note_comment.rb [new file with mode: 0644]
db/structure.sql
test/fixtures/note_comments.yml
test/functional/notes_controller_test.rb

index 8b95fea2e3bdd75c578b83016e0ba74e667b5760..d85d7859250fd1d73162e692a5d196dce5a8a9bd 100644 (file)
@@ -3,9 +3,15 @@
   <% note.comments.forEach(function (comment) { %>
   <p>
     <small class="deemphasize">
-      <%- I18n.t('javascripts.notes.show.event', {
-         action: comment.action, user: comment.user, time: comment.date
-      }) %>
+      <% if (comment.user) { %>
+        <%- I18n.t('javascripts.notes.show.event', {
+           action: comment.action, user: comment.user, time: comment.date
+        }) %>
+      <% } else { %>
+        <%- I18n.t('javascripts.notes.show.anonymous_event', {
+           action: comment.action, time: comment.date
+        }) %>
+      <% } %>
     </small>
     <br/>
     <%- comment.text %>
index 5a09342474e25906c2489df86604e8ff2cc233ca..d25d9a37e9a756d39dd5a438cb3dbc2858fb7605 100644 (file)
@@ -58,7 +58,6 @@ class NotesController < ApplicationController
     lon = params[:lon].to_f
     lat = params[:lat].to_f
     comment = params[:text]
-    name = params[:name]
 
     # Include in a transaction to ensure that there is always a note_comment for every note
     Note.transaction do
@@ -70,7 +69,7 @@ class NotesController < ApplicationController
       @note.save!
 
       # Add a comment to the note
-      add_comment(@note, comment, name, "opened")
+      add_comment(@note, comment, "opened")
     end
 
     # Return a copy of the new note
@@ -90,7 +89,6 @@ class NotesController < ApplicationController
     # Extract the arguments
     id = params[:id].to_i
     comment = params[:text]
-    name = params[:name] or "NoName"
 
     # Find the note and check it is valid
     @note = Note.find(id)
@@ -99,7 +97,7 @@ class NotesController < ApplicationController
 
     # Add a comment to the note
     Note.transaction do
-      add_comment(@note, comment, name, "commented")
+      add_comment(@note, comment, "commented")
     end
 
     # Return a copy of the updated note
@@ -118,7 +116,6 @@ class NotesController < ApplicationController
     # Extract the arguments
     id = params[:id].to_i
     comment = params[:text]
-    name = params[:name]
 
     # Find the note and check it is valid
     @note = Note.find_by_id(id)
@@ -129,7 +126,7 @@ class NotesController < ApplicationController
     Note.transaction do
       @note.close
 
-      add_comment(@note, comment, name, "closed")
+      add_comment(@note, comment, "closed")
     end
 
     # Return a copy of the updated note
@@ -192,7 +189,6 @@ class NotesController < ApplicationController
 
     # Extract the arguments
     id = params[:id].to_i
-    name = params[:name]
 
     # Find the note and check it is valid
     note = Note.find(id)
@@ -204,7 +200,7 @@ class NotesController < ApplicationController
       note.status = "hidden"
       note.save
 
-      add_comment(note, nil, name, "hidden")
+      add_comment(note, nil, "hidden")
     end
 
     # Render the result
@@ -302,17 +298,13 @@ private
 
   ##
   # Add a comment to a note
-  def add_comment(note, text, name, event)
-    name = "NoName" if name.nil?
-
+  def add_comment(note, text, event)
     attributes = { :visible => true, :event => event, :body => text }
 
     if @user  
       attributes[:author_id] = @user.id
-      attributes[:author_name] = @user.display_name
     else  
       attributes[:author_ip] = request.remote_ip
-      attributes[:author_name] = name + " (a)"
     end
 
     comment = note.comments.create(attributes, :without_protection => true)
index 18f233e506092d05d9e89ac302b8376ff2d021cb..e32d1bb6ead572bace0b7d64a7c86f1776b9f087 100644 (file)
@@ -102,9 +102,9 @@ module ApplicationHelper
 
   def note_author(object, link_options = {})
     if object.author.nil?
-      h(object.author_name)
+      ""
     else
-      link_to h(object.author_name), link_options.merge({:controller => "user", :action => "view", :display_name => object.author_name})
+      link_to h(object.author.display_name), link_options.merge({:controller => "user", :action => "view", :display_name => object.author.display_name})
     end
   end
 end
index c32b1679b07b593f1dab2c8e9045e48cb1796e71..14806be266dfd1114e62ad324a97708aafc8e65b 100644 (file)
@@ -39,7 +39,7 @@ class Note < ActiveRecord::Base
       resp += (comment_no == 1 ? "" : separator_char)
       resp += comment.body if comment.body
       resp += " [ " 
-      resp += comment.author_name if comment.author_name
+      resp += comment.author.display_name if comment.author
       resp += " " + comment.created_at.to_s + " ]"
       comment_no += 1
     end
@@ -62,16 +62,6 @@ class Note < ActiveRecord::Base
     self.comments.first.author_ip
   end
 
-  # Return the author id, derived from the first comment
-  def author_id
-    self.comments.first.author_id
-  end
-
-  # Return the author name, derived from the first comment
-  def author_name
-    self.comments.first.author_name
-  end
-
 private
 
   # Fill in default values for new notes
index 0500acb8bf2b11ffb12eda1c54bac03210ef5143..39902911879abffbd0d455f91bcd2b524ac24f52 100644 (file)
@@ -10,15 +10,6 @@ class NoteComment < ActiveRecord::Base
   validates_associated :author
   validates_inclusion_of :event, :in => [ "opened", "closed", "reopened", "commented", "hidden" ]
 
-  # Return the author name
-  def author_name
-    if self.author_id.nil?
-      self.read_attribute(:author_name)
-    else
-      self.author.display_name
-    end
-  end
-
   # Return the comment text
   def body
     RichText.new("text", read_attribute(:body))
index ae50fa9b16e1fc339e1721939a6279d22d8aa415..5972a700b8af794c9246c777df171a458e445820 100644 (file)
@@ -6,7 +6,7 @@ class Notifier < ActionMailer::Base
 
   def signup_confirm(user, token)
     @locale = user.preferred_language_from(I18n.available_locales)
-    
+
     # If we are passed an email address verification token, create
     # the confirumation URL for account activation.
     #
@@ -19,7 +19,7 @@ class Notifier < ActionMailer::Base
                      :display_name => user.display_name,
                      :confirm_string => token.token)
     end
-      
+
     mail :to => user.email,
          :subject => I18n.t('notifier.signup_confirm.subject', :locale => @locale)
   end
@@ -67,7 +67,7 @@ class Notifier < ActionMailer::Base
     mail :to => trace.user.email,
          :subject => I18n.t('notifier.gpx_notification.failure.subject', :locale => @locale)
   end
-  
+
   def message_notification(message)
     @locale = message.recipient.preferred_language_from(I18n.available_locales)
     @to_user = message.recipient.display_name
@@ -129,10 +129,18 @@ class Notifier < ActionMailer::Base
     @place = Nominatim.describe_location(comment.note.lat, comment.note.lon, 14, @locale)
     @comment = comment.body
     @owner = recipient == comment.note.author
-    @commenter = comment.author_name
 
-    subject = I18n.t('notifier.note_comment_notification.subject_own', :commenter => comment.author_name) if @owner
-    subject = I18n.t('notifier.note_comment_notification.subject_other', :commenter => comment.author_name) unless @owner
+    if comment.author
+      @commenter = comment.author.display_name
+    else
+      @commenter = I18n.t("notifier.note_comment_notification.anonymous")
+    end
+
+    if @owner
+      subject = I18n.t('notifier.note_comment_notification.subject_own', :commenter => @commenter)
+    else
+      subject = I18n.t('notifier.note_comment_notification.subject_other', :commenter => @commenter)
+    end
 
     mail :to => recipient.email, :subject => subject
   end
index f252a0dfc41b930c4e00e4aae0503f3eebe14f78..ce9ddb4bfc8c6a122355baeac9fe97c02f7e905b 100644 (file)
@@ -16,8 +16,8 @@ json.properties do
 
   json.comments(note.comments) do |comment|
     json.date comment.created_at
-    json.uid comment.author_id unless comment.author_id.nil?
-    json.user comment.author_name
+    json.uid comment.author.id unless comment.author.nil?
+    json.user comment.author.display_name unless comment.author.nil?
     json.action comment.event
     json.text comment.body unless comment.body.nil?
   end
index cffc3f271c2d75ddde6adaf070aebdffae15825c..fbd217beb75f94e1b9801ef724cac75aadb675fc 100644 (file)
@@ -12,7 +12,11 @@ xml.item do
   xml.link browse_note_url(note)
   xml.guid note_url(note)
   xml.description render(:partial => "description", :object => note, :formats => [ :html ])
-  xml.author note.comments.first.author_name
+
+  if note.author
+    xml.author note.author_display_name
+  end
+
   xml.pubDate note.updated_at.to_s(:rfc822)
   xml.geo :lat, note.lat
   xml.geo :long, note.lon
index a68946bb7a7e46ff9eeb83594052f3fb238ee183..1877726769a968f78c74c07ca0dd803081239ef3 100644 (file)
@@ -14,8 +14,8 @@ xml.note("lon" => note.lon, "lat" => note.lat) do
     note.comments.each do |comment|
       xml.comment do
         xml.date comment.created_at
-        xml.uid comment.author_id unless comment.author_id.nil?
-        xml.user comment.author_name
+        xml.uid comment.author.id unless comment.author.nil?
+        xml.user comment.author.display_name unless comment.author.nil?
         xml.text comment.body
       end      
     end
index 7113364f01fb2dffa59885529afdde8982267cc7..4733c1bad77864377814f32139676c1ab27175e9 100644 (file)
@@ -37,7 +37,11 @@ xml.rss("version" => "2.0",
         description_text += comment.note.flatten_comment("<br>", comment.created_at)
 
         xml.description description_text 
-        xml.author comment.author_name
+
+        if comment.author
+          xml.author comment.author.display_name
+        end
+
         xml.pubDate comment.created_at.to_s(:rfc822)
         xml.geo :lat, comment.note.lat
         xml.geo :long, comment.note.lon
index c58548b47c385aa2cc32bf8a5f26edce345877e0..e385e942baad0c42473d0824defa8e0850a47450 100644 (file)
@@ -23,9 +23,9 @@
     </td>
     <td><%= link_to note.id.to_s, :controller => "browse", :action => "note", :id => note.id %></td>
     <% if note.author.nil? %> 
-      <td> <%= note.author_name %> </td> 
+      <td></td> 
     <% else %> 
-      <td><%= link_to h(note.author_name), :controller => "user", :action => "view", :display_name => note.author_name %></td>
+      <td><%= link_to h(note.author.display_name), user_url(:display_name => note.author.display_name) %></td>
     <% end %>
     <td> <%= note.comments.first.body.to_html  %> </td>        
     <td><%= l note.created_at %></td>
index 62b6b90f846858bfd8dfdc9c00901b0aaf84fbc2..605ff4381d5937683d67d8d1dde143a1a2935862 100644 (file)
@@ -1204,6 +1204,7 @@ en:
       hopefully_you: "Someone (possibly you) has asked for the password to be reset on this email address's openstreetmap.org account."
       click_the_link: "If this is you, please click the link below to reset your password."
     note_comment_notification:
+      anonymous: An anonymous user
       subject_own: "[OpenStreetMap] %{commenter} has commented on one of your notes"
       subject_other: "[OpenStreetMap] %{commenter} has commented on a note you are interested in"
       greeting: "Hi,"
@@ -2022,6 +2023,7 @@ en:
       show:
         title: Note %{id}
         event: "%{action} by %{user} at %{time}"
+        anonymous_event: "%{action} by anonymous at %{time}"
         close: Close
         comment_and_close: Comment & Close
         comment: Comment
diff --git a/db/migrate/20121202155309_remove_author_name_from_note_comment.rb b/db/migrate/20121202155309_remove_author_name_from_note_comment.rb
new file mode 100644 (file)
index 0000000..60d0ab2
--- /dev/null
@@ -0,0 +1,9 @@
+class RemoveAuthorNameFromNoteComment < ActiveRecord::Migration
+  def up
+    remove_column :note_comments, :author_name
+  end
+
+  def down
+    add_column :note_comments, :author_name, :string
+  end
+end
index 84c38c86ef277054fd6adbfd352874b9f4ffa504..eaf279f9c933cb72087bd688aee5a632e01e6dcd 100644 (file)
@@ -733,7 +733,6 @@ CREATE TABLE note_comments (
     note_id bigint NOT NULL,
     visible boolean NOT NULL,
     created_at timestamp without time zone NOT NULL,
-    author_name character varying(255),
     author_ip character varying(255),
     author_id bigint,
     body text,
@@ -771,7 +770,6 @@ CREATE TABLE notes (
     tile bigint NOT NULL,
     updated_at timestamp without time zone NOT NULL,
     created_at timestamp without time zone NOT NULL,
-    nearby_place character varying(255),
     status note_status_enum NOT NULL,
     closed_at timestamp without time zone
 );
@@ -2447,6 +2445,10 @@ INSERT INTO schema_migrations (version) VALUES ('20121005195010');
 
 INSERT INTO schema_migrations (version) VALUES ('20121012044047');
 
+INSERT INTO schema_migrations (version) VALUES ('20121119165817');
+
+INSERT INTO schema_migrations (version) VALUES ('20121202155309');
+
 INSERT INTO schema_migrations (version) VALUES ('21');
 
 INSERT INTO schema_migrations (version) VALUES ('22');
index e078b990ad2b56a17a612f6c3cf15677f3e47783..881adb6ace874f89ffeee0adb4c49b4768e44e18 100644 (file)
@@ -3,7 +3,6 @@ t1:
   note_id: 1
   visible: true
   created_at: 2007-01-01 00:00:00
-  author_name: 'testname'
   author_ip: '192.168.1.1'
   body: 'This is the initial description of the note 1'
 
@@ -12,7 +11,6 @@ t2:
   note_id: 2
   visible: true
   created_at: 2007-01-01 00:00:00
-  author_name: 'testname'
   author_ip: '192.168.1.1'
   body: 'This is the initial description of the note 2'
 
@@ -21,7 +19,6 @@ t3:
   note_id: 2
   visible: true
   created_at: 2007-02-01 00:00:00
-  author_name: 'testname'
   author_ip: '192.168.1.1'
   body: 'This is an additional comment for note 2'
 
@@ -30,7 +27,6 @@ t4:
   note_id: 3
   visible: true
   created_at: 2007-01-01 00:00:00
-  author_name: 'testname'
   author_ip: '192.168.1.1'
   body: 'This is the initial comment for note 3'
 
@@ -39,7 +35,6 @@ t5:
   note_id: 4
   visible: true
   created_at: 2007-01-01 00:00:00
-  author_name: 'testname'
   author_ip: '192.168.1.1'
   body: 'Spam for note 4'
 
@@ -48,7 +43,6 @@ t6:
   note_id: 5
   visible: true
   created_at: 2007-01-01 00:00:00
-  author_name: 'testname'
   author_ip: '192.168.1.1'
   body: 'Valid comment for note 5'
 
@@ -57,7 +51,6 @@ t7:
   note_id: 5
   visible: false
   created_at: 2007-02-01 00:00:00
-  author_name: 'testname'
   author_ip: '192.168.1.1'
   body: 'Spam for note 5'
 
@@ -66,7 +59,6 @@ t8:
   note_id: 5
   visible: true
   created_at: 2007-02-01 00:00:00
-  author_name: 'testname'
   author_ip: '192.168.1.1'
   body: 'Another valid comment for note 5'
 
@@ -94,7 +86,6 @@ t11:
   visible: true
   event: opened
   created_at: 2007-01-01 00:00:00
-  author_name: 'testname'
   author_ip: '192.168.1.1'
   body: 'Initial note description'
 
@@ -104,7 +95,6 @@ t12:
   visible: true
   event: commented
   created_at: 2007-02-01 00:00:00
-  author_name: 'testname'
   author_ip: '192.168.1.1'
   body: 'A comment description'
 
index 00e887f33473072095eeaa41223bc4bc3fc5e59c..1c0ded3558f567a8417a53bbe20fbe8f85a0a6a0 100644 (file)
@@ -120,7 +120,7 @@ class NotesControllerTest < ActionController::TestCase
   def test_note_create_success
     assert_difference('Note.count') do
       assert_difference('NoteComment.count') do
-        post :create, {:lat => -1.0, :lon => -1.0, :name => "new_tester", :text => "This is a comment", :format => "json"}
+        post :create, {:lat => -1.0, :lon => -1.0, :text => "This is a comment", :format => "json"}
       end
     end
     assert_response :success
@@ -133,7 +133,7 @@ class NotesControllerTest < ActionController::TestCase
     assert_equal 1, js["properties"]["comments"].count
     assert_equal "opened", js["properties"]["comments"].last["action"]
     assert_equal "This is a comment", js["properties"]["comments"].last["text"]
-    assert_equal "new_tester (a)", js["properties"]["comments"].last["user"]
+    assert_nil js["properties"]["comments"].last["user"]
     id = js["properties"]["id"]
 
     get :show, {:id => id, :format => "json"}
@@ -148,41 +148,41 @@ class NotesControllerTest < ActionController::TestCase
     assert_equal 1, js["properties"]["comments"].count
     assert_equal "opened", js["properties"]["comments"].last["action"]
     assert_equal "This is a comment", js["properties"]["comments"].last["text"]
-    assert_equal "new_tester (a)", js["properties"]["comments"].last["user"]
+    assert_nil js["properties"]["comments"].last["user"]
   end
 
   def test_note_create_fail
     assert_no_difference('Note.count') do
       assert_no_difference('NoteComment.count') do
-        post :create, {:lon => -1.0, :name => "new_tester", :text => "This is a comment"}
+        post :create, {:lon => -1.0, :text => "This is a comment"}
       end
     end
     assert_response :bad_request
 
     assert_no_difference('Note.count') do
       assert_no_difference('NoteComment.count') do
-        post :create, {:lat => -1.0, :name => "new_tester", :text => "This is a comment"}
+        post :create, {:lat => -1.0, :text => "This is a comment"}
       end
     end
     assert_response :bad_request
 
     assert_no_difference('Note.count') do
       assert_no_difference('NoteComment.count') do
-        post :create, {:lat => -1.0, :lon => -1.0, :name => "new_tester"}
+        post :create, {:lat => -1.0, :lon => -1.0}
       end
     end
     assert_response :bad_request
 
     assert_no_difference('Note.count') do
       assert_no_difference('NoteComment.count') do
-        post :create, {:lat => -100.0, :lon => -1.0, :name => "new_tester", :text => "This is a comment"}
+        post :create, {:lat => -100.0, :lon => -1.0, :text => "This is a comment"}
       end
     end
     assert_response :bad_request
 
     assert_no_difference('Note.count') do
       assert_no_difference('NoteComment.count') do
-        post :create, {:lat => -1.0, :lon => -200.0, :name => "new_tester", :text => "This is a comment"}
+        post :create, {:lat => -1.0, :lon => -200.0, :text => "This is a comment"}
       end
     end
     assert_response :bad_request
@@ -190,7 +190,7 @@ class NotesControllerTest < ActionController::TestCase
 
   def test_note_comment_create_success
     assert_difference('NoteComment.count') do
-      post :comment, {:id => notes(:open_note_with_comment).id, :name => "new_tester2", :text => "This is an additional comment", :format => "json"}
+      post :comment, {:id => notes(:open_note_with_comment).id, :text => "This is an additional comment", :format => "json"}
     end
     assert_response :success
     js = ActiveSupport::JSON.decode(@response.body)
@@ -201,7 +201,7 @@ class NotesControllerTest < ActionController::TestCase
     assert_equal 3, js["properties"]["comments"].count
     assert_equal "commented", js["properties"]["comments"].last["action"]
     assert_equal "This is an additional comment", js["properties"]["comments"].last["text"]
-    assert_equal "new_tester2 (a)", js["properties"]["comments"].last["user"]
+    assert_nil js["properties"]["comments"].last["user"]
 
     get :show, {:id => notes(:open_note_with_comment).id, :format => "json"}
     assert_response :success
@@ -213,27 +213,27 @@ class NotesControllerTest < ActionController::TestCase
     assert_equal 3, js["properties"]["comments"].count
     assert_equal "commented", js["properties"]["comments"].last["action"]
     assert_equal "This is an additional comment", js["properties"]["comments"].last["text"]
-    assert_equal "new_tester2 (a)", js["properties"]["comments"].last["user"]
+    assert_nil js["properties"]["comments"].last["user"]
   end
 
   def test_note_comment_create_fail
     assert_no_difference('NoteComment.count') do
-      post :comment, {:name => "new_tester2", :text => "This is an additional comment"}
+      post :comment, {:text => "This is an additional comment"}
     end
     assert_response :bad_request
 
     assert_no_difference('NoteComment.count') do
-      post :comment, {:id => notes(:open_note_with_comment).id, :name => "new_tester2"}
+      post :comment, {:id => notes(:open_note_with_comment).id}
     end
     assert_response :bad_request
 
     assert_no_difference('NoteComment.count') do
-      post :comment, {:id => 12345, :name => "new_tester2", :text => "This is an additional comment"}
+      post :comment, {:id => 12345, :text => "This is an additional comment"}
     end
     assert_response :not_found
 
     assert_no_difference('NoteComment.count') do
-      post :comment, {:id => notes(:hidden_note_with_comment).id, :name => "new_tester2", :text => "This is an additional comment"}
+      post :comment, {:id => notes(:hidden_note_with_comment).id, :text => "This is an additional comment"}
     end
     assert_response :gone
   end
@@ -249,7 +249,7 @@ class NotesControllerTest < ActionController::TestCase
     assert_equal 3, js["properties"]["comments"].count
     assert_equal "closed", js["properties"]["comments"].last["action"]
     assert_equal "This is a close comment", js["properties"]["comments"].last["text"]
-    assert_equal "NoName (a)", js["properties"]["comments"].last["user"]
+    assert_nil js["properties"]["comments"].last["user"]
 
     get :show, {:id => notes(:open_note_with_comment).id, :format => "json"}
     assert_response :success
@@ -261,7 +261,7 @@ class NotesControllerTest < ActionController::TestCase
     assert_equal 3, js["properties"]["comments"].count
     assert_equal "closed", js["properties"]["comments"].last["action"]
     assert_equal "This is a close comment", js["properties"]["comments"].last["text"]
-    assert_equal "NoName (a)", js["properties"]["comments"].last["user"]
+    assert_nil js["properties"]["comments"].last["user"]
   end
 
   def test_note_close_fail