From: Tom Hughes Date: Sun, 18 Sep 2011 13:26:59 +0000 (+0100) Subject: Merge branch 'master' into openstreetbugs X-Git-Tag: live~5120^2~129 X-Git-Url: https://git.openstreetmap.org/rails.git/commitdiff_plain/3a654c89197b40f4a623c0dd33b87b415623d97d?hp=3259bc7edecbcfc54eb70e3c0a1e07d6b582a46c Merge branch 'master' into openstreetbugs --- diff --git a/app/controllers/browse_controller.rb b/app/controllers/browse_controller.rb index a7dd5f5c9..2036e4f10 100644 --- a/app/controllers/browse_controller.rb +++ b/app/controllers/browse_controller.rb @@ -79,4 +79,13 @@ class BrowseController < ApplicationController rescue ActiveRecord::RecordNotFound render :action => "not_found", :status => :not_found end + + def note + @type = "note" + @note = Note.find(params[:id]) + @next = Note.find(:first, :order => "id ASC", :conditions => [ "status != 'hidden' AND id > :id", { :id => @note.id }] ) + @prev = Note.find(:first, :order => "id DESC", :conditions => [ "status != 'hidden' AND id < :id", { :id => @note.id }] ) + rescue ActiveRecord::RecordNotFound + render :action => "not_found", :status => :not_found + end end diff --git a/app/controllers/note_controller.rb b/app/controllers/note_controller.rb new file mode 100644 index 000000000..c46145045 --- /dev/null +++ b/app/controllers/note_controller.rb @@ -0,0 +1,387 @@ +class NoteController < ApplicationController + + layout 'site', :only => [:mine] + + before_filter :check_api_readable + before_filter :authorize_web, :only => [:create, :close, :update, :delete, :mine] + before_filter :check_api_writable, :only => [:create, :close, :update, :delete] + before_filter :set_locale, :only => [:mine] + after_filter :compress_output + around_filter :api_call_handle_error, :api_call_timeout + + # Help methods for checking boundary sanity and area size + include MapBoundary + + ## + # Return a list of notes in a given area + def list + # Figure out the bbox - we prefer a bbox argument but also + # support the old, deprecated, method with four arguments + if params[:bbox] + raise OSM::APIBadUserInput.new("Invalid bbox") unless params[:bbox].count(",") == 3 + + bbox = params[:bbox].split(",") + else + raise OSM::APIBadUserInput.new("No l was given") unless params[:l] + raise OSM::APIBadUserInput.new("No r was given") unless params[:r] + raise OSM::APIBadUserInput.new("No b was given") unless params[:b] + raise OSM::APIBadUserInput.new("No t was given") unless params[:t] + + bbox = [ params[:l], params[:b], params[:r], params[:t] ] + end + + # Get the sanitised boundaries + @min_lon, @min_lat, @max_lon, @max_lat = sanitise_boundaries(bbox) + + # Get any conditions that need to be applied + conditions = closed_condition + + # Check that the boundaries are valid + check_boundaries(@min_lon, @min_lat, @max_lon, @max_lat, MAX_NOTE_REQUEST_AREA) + + # Find the notes we want to return + @notes = Note.find_by_area(@min_lat, @min_lon, @max_lat, @max_lon, + :include => :comments, + :conditions => conditions, + :order => "updated_at DESC", + :limit => result_limit) + + # Render the result + respond_to do |format| + format.html { render :format => :rjs, :content_type => "text/javascript" } + format.rss + format.js + format.xml + format.json { render :json => @notes.to_json } + format.gpx + end + end + + ## + # Create a new note + def create + # Check the arguments are sane + raise OSM::APIBadUserInput.new("No lat was given") unless params[:lat] + raise OSM::APIBadUserInput.new("No lon was given") unless params[:lon] + raise OSM::APIBadUserInput.new("No text was given") unless params[:text] + + # Extract the arguments + 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 + # Create the note + @note = Note.create(:lat => lat, :lon => lon) + raise OSM::APIBadUserInput.new("The note is outside this world") unless @note.in_world? + + #TODO: move this into a helper function + begin + url = "http://nominatim.openstreetmap.org/reverse?lat=" + lat.to_s + "&lon=" + lon.to_s + "&zoom=16" + response = REXML::Document.new(Net::HTTP.get(URI.parse(url))) + + if result = response.get_text("reversegeocode/result") + @note.nearby_place = result.to_s + else + @note.nearby_place = "unknown" + end + rescue Exception => err + @note.nearby_place = "unknown" + end + + # Save the note + @note.save + + # Add a comment to the note + add_comment(@note, comment, name, "opened") + end + + # Send an OK response + render_ok + end + + ## + # Add a comment to an existing note + def update + # Check the arguments are sane + raise OSM::APIBadUserInput.new("No id was given") unless params[:id] + raise OSM::APIBadUserInput.new("No text was given") unless params[:text] + + # 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) + raise OSM::APINotFoundError unless note + raise OSM::APIAlreadyDeletedError unless note.visible? + + # Add a comment to the note + Note.transaction do + add_comment(note, comment, name, "commented") + end + + # Send an OK response + render_ok + end + + ## + # Close a note + def close + # Check the arguments are sane + raise OSM::APIBadUserInput.new("No id was given") unless params[:id] + + # Extract the arguments + id = params[:id].to_i + name = params[:name] + + # Find the note and check it is valid + note = Note.find_by_id(id) + raise OSM::APINotFoundError unless note + raise OSM::APIAlreadyDeletedError unless note.visible? + + # Close the note and add a comment + Note.transaction do + note.close + + add_comment(note, nil, name, "closed") + end + + # Send an OK response + render_ok + end + + ## + # Get a feed of recent notes and comments + def rss + # Get any conditions that need to be applied + conditions = closed_condition + + # Process any bbox + if params[:bbox] + raise OSM::APIBadUserInput.new("Invalid bbox") unless params[:bbox].count(",") == 3 + + @min_lon, @min_lat, @max_lon, @max_lat = sanitise_boundaries(params[:bbox].split(',')) + + check_boundaries(@min_lon, @min_lat, @max_lon, @max_lat, MAX_NOTE_REQUEST_AREA) + + conditions = cond_merge conditions, [OSM.sql_for_area(@min_lat, @min_lon, @max_lat, @max_lon, "notes.")] + end + + # Find the comments we want to return + @comments = NoteComment.find(:all, + :conditions => conditions, + :order => "created_at DESC", + :limit => result_limit, + :joins => :note, + :include => :note) + + # Render the result + respond_to do |format| + format.rss + end + end + + ## + # Read a note + def read + # Check the arguments are sane + raise OSM::APIBadUserInput.new("No id was given") unless params[:id] + + # Find the note and check it is valid + @note = Note.find(params[:id]) + raise OSM::APINotFoundError unless @note + raise OSM::APIAlreadyDeletedError unless @note.visible? + + # Render the result + respond_to do |format| + format.xml + format.rss + format.json { render :json => @note.to_json } + format.gpx + end + end + + ## + # Delete (hide) a note + def delete + # Check the arguments are sane + raise OSM::APIBadUserInput.new("No id was given") unless params[:id] + + # Extract the arguments + id = params[:id].to_i + name = params[:name] + + # Find the note and check it is valid + note = Note.find(id) + raise OSM::APINotFoundError unless note + raise OSM::APIAlreadyDeletedError unless note.visible? + + # Mark the note as hidden + Note.transaction do + note.status = "hidden" + note.save + + add_comment(note, nil, name, "hidden") + end + + # Render the result + render :text => "ok\n", :content_type => "text/html" + end + + ## + # Return a list of notes matching a given string + def search + # Check the arguments are sane + raise OSM::APIBadUserInput.new("No query string was given") unless params[:q] + + # Get any conditions that need to be applied + conditions = closed_condition + conditions = cond_merge conditions, ['note_comments.body ~ ?', params[:q]] + + # Find the notes we want to return + @notes = Note.find(:all, + :conditions => conditions, + :order => "updated_at DESC", + :limit => result_limit, + :joins => :comments, + :include => :comments) + + # Render the result + respond_to do |format| + format.html { render :action => :list, :format => :rjs, :content_type => "text/javascript"} + format.rss { render :action => :list } + format.js + format.xml { render :action => :list } + format.json { render :json => @notes.to_json } + format.gpx { render :action => :list } + end + end + + def mine + if params[:display_name] + @user2 = User.find_by_display_name(params[:display_name], :conditions => { :status => ["active", "confirmed"] }) + + if @user2 + if @user2.data_public? or @user2 == @user + conditions = ['note_comments.author_id = ?', @user2.id] + else + conditions = ['false'] + end + else #if request.format == :html + @title = t 'user.no_such_user.title' + @not_found_user = params[:display_name] + render :template => 'user/no_such_user', :status => :not_found + return + end + end + + if @user2 + user_link = render_to_string :partial => "user", :object => @user2 + end + + @title = t 'note.mine.title', :user => @user2.display_name + @heading = t 'note.mine.heading', :user => @user2.display_name + @description = t 'note.mine.description', :user => user_link + + @page = (params[:page] || 1).to_i + @page_size = 10 + + @notes = Note.find(:all, + :include => [:comments, {:comments => :author}], + :joins => :comments, + :order => "updated_at DESC", + :conditions => conditions, + :offset => (@page - 1) * @page_size, + :limit => @page_size).uniq + end + +private + #------------------------------------------------------------ + # utility functions below. + #------------------------------------------------------------ + + ## + # merge two conditions + # TODO: this is a copy from changeset_controler.rb and should be factored out to share + def cond_merge(a, b) + if a and b + a_str = a.shift + b_str = b.shift + return [ a_str + " AND " + b_str ] + a + b + elsif a + return a + else b + return b + end + end + + ## + # Render an OK response + def render_ok + if params[:format] == "js" + render :text => "osbResponse();", :content_type => "text/javascript" + else + render :text => "ok " + @note.id.to_s + "\n", :content_type => "text/plain" if @note + render :text => "ok\n", :content_type => "text/plain" unless @note + end + end + + ## + # Get the maximum number of results to return + def result_limit + if params[:limit] and params[:limit].to_i > 0 and params[:limit].to_i < 10000 + params[:limit].to_i + else + 100 + end + end + + ## + # Generate a condition to choose which bugs we want based + # on their status and the user's request parameters + def closed_condition + if params[:closed] + closed_since = params[:closed].to_i + else + closed_since = 7 + end + + if closed_since < 0 + conditions = ["status != 'hidden'"] + elsif closed_since > 0 + conditions = ["(status = 'open' OR (status = 'closed' AND closed_at > '#{Time.now - closed_since.days}'))"] + else + conditions = ["status = 'open'"] + end + + return conditions + end + + ## + # Add a comment to a note + def add_comment(note, text, name, event) + name = "NoName" if name.nil? + + 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 + + note.comments.create(attributes) + + note.comments.map { |c| c.author }.uniq.each do |user| + if user and user != @user + Notifier.deliver_note_comment_notification(comment, user) + end + end + end +end diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 6e2ecd323..ec6c455b3 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -129,6 +129,26 @@ module ApplicationHelper end end + def friendly_date(date) + content_tag(:span, time_ago_in_words(date), :title => l(date, :format => :friendly)) + end + + 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}) + end + end + + def with_format(format, &block) + old_format = @template_format + @template_format = format + result = block.call + @template_format = old_format + return result + end + private def javascript_strings_for_key(key) diff --git a/app/models/note.rb b/app/models/note.rb new file mode 100644 index 000000000..892ada1aa --- /dev/null +++ b/app/models/note.rb @@ -0,0 +1,88 @@ +class Note < ActiveRecord::Base + include GeoRecord + + has_many :comments, :class_name => "NoteComment", + :foreign_key => :note_id, + :order => :created_at, + :conditions => { :visible => true } + + validates_presence_of :id, :on => :update + validates_uniqueness_of :id + validates_numericality_of :latitude, :only_integer => true + validates_numericality_of :longitude, :only_integer => true + validates_presence_of :closed_at if :status == "closed" + validates_inclusion_of :status, :in => ["open", "closed", "hidden"] + validate :validate_position + + # Sanity check the latitude and longitude and add an error if it's broken + def validate_position + errors.add_to_base("Note is not in the world") unless in_world? + end + + # Fill in default values for new notes + def after_initialize + self.status = "open" unless self.attribute_present?(:status) + end + + # Close a note + def close + self.status = "closed" + self.closed_at = Time.now.getutc + self.save + end + + # Return a flattened version of the comments for a note + def flatten_comment(separator_char, upto_timestamp = :nil) + resp = "" + comment_no = 1 + self.comments.each do |comment| + next if upto_timestamp != :nil and comment.created_at > upto_timestamp + resp += (comment_no == 1 ? "" : separator_char) + resp += comment.body if comment.body + resp += " [ " + resp += comment.author_name if comment.author_name + resp += " " + comment.created_at.to_s + " ]" + comment_no += 1 + end + + return resp + end + + # Check if a note is visible + def visible? + return status != "hidden" + end + + # Return the author object, derived from the first comment + def author + self.comments.first.author + end + + # Return the author IP address, derived from the first comment + def author_ip + 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 + + # Custom JSON output routine for notes + def to_json(options = {}) + super options.reverse_merge( + :methods => [ :lat, :lon ], + :only => [ :id, :status, :created_at ], + :include => { + :comments => { + :only => [ :event, :author_name, :created_at, :body ] + } + } + ) + end +end diff --git a/app/models/note_comment.rb b/app/models/note_comment.rb new file mode 100644 index 000000000..bcbcf79be --- /dev/null +++ b/app/models/note_comment.rb @@ -0,0 +1,21 @@ +class NoteComment < ActiveRecord::Base + belongs_to :note, :foreign_key => :note_id + belongs_to :author, :class_name => "User", :foreign_key => :author_id + + validates_presence_of :id, :on => :update + validates_uniqueness_of :id + validates_presence_of :note_id + validates_associated :note + validates_presence_of :visible + 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 +end diff --git a/app/models/notifier.rb b/app/models/notifier.rb index e6058d4b7..f025da7b1 100644 --- a/app/models/notifier.rb +++ b/app/models/notifier.rb @@ -95,6 +95,22 @@ class Notifier < ActionMailer::Base body :friend => friend end + def note_comment_notification(comment, recipient) + common_headers recipient + owner = (recipient == comment.note.author); + subject I18n.t('notifier.note_plain.subject_own', :commenter => comment.author_name) if owner + subject I18n.t('notifier.note_plain.subject_other', :commenter => comment.author_name) unless owner + + body :nodeurl => url_for(:host => SERVER_URL, + :controller => "browse", + :action => "note", + :id => comment.note_id), + :place => comment.note.nearby_place, + :comment => comment.body, + :owner => owner, + :commenter => comment.author_name + end + private def common_headers(recipient) diff --git a/app/views/browse/_map.html.erb b/app/views/browse/_map.html.erb index 1ff86cd4e..2d04efe04 100644 --- a/app/views/browse/_map.html.erb +++ b/app/views/browse/_map.html.erb @@ -6,14 +6,18 @@
- <% if map.instance_of? Changeset or (map.instance_of? Node and map.version > 1) or map.visible %> + <% if map.instance_of? Changeset or (map.instance_of? Node and map.version > 1) or map.visible? %>
<%= t 'browse.map.loading' %> + <% if map.instance_of? Note -%> + <%= link_to(t("browse.map.larger.area"), { :controller => :site, :action => :index, :notes => "yes" }, { :id => "area_larger_map", :class => "geolink bbox" }) %> + <% else -%> <%= link_to(t("browse.map.larger.area"), { :controller => :site, :action => :index, :box => "yes" }, { :id => "area_larger_map", :class => "geolink bbox" }) %> + <% end -%>
<%= link_to(t("browse.map.edit.area"), { :controller => :site, :action => :edit }, { :id => "area_edit", :class => "geolink bbox" }) %> - <% unless map.instance_of? Changeset %> + <% unless map.instance_of? Changeset or map.instance_of? Note %>
<%= link_to("", { :controller => :site, :action => :index }, { :id => "object_larger_map", :class => "geolink object" }) %>
@@ -40,7 +44,7 @@
-<% if map.instance_of? Changeset or (map.instance_of? Node and map.version > 1) or map.visible %> +<% if map.instance_of? Changeset or (map.instance_of? Node and map.version > 1) or map.visible? %>