From 9df991e343d55776c0e645e373110f8039593d04 Mon Sep 17 00:00:00 2001 From: Tom Hughes Date: Sun, 7 Jun 2009 16:59:58 +0000 Subject: [PATCH 1/1] Marge all the changeset list methods together into one. --- app/controllers/changeset_controller.rb | 95 +++++++------------ .../changeset/_changeset_paging_nav.rhtml | 2 + app/views/changeset/list.rhtml | 15 ++- app/views/changeset/list.rxml | 9 +- app/views/changeset/list_bbox.rhtml | 51 ---------- app/views/changeset/list_bbox.rxml | 48 ---------- app/views/changeset/list_user.rhtml | 18 ---- app/views/changeset/list_user.rxml | 48 ---------- config/locales/en.yml | 34 +------ config/routes.rb | 10 +- test/functional/changeset_controller_test.rb | 10 +- 11 files changed, 62 insertions(+), 278 deletions(-) delete mode 100644 app/views/changeset/list_bbox.rhtml delete mode 100644 app/views/changeset/list_bbox.rxml delete mode 100644 app/views/changeset/list_user.rhtml delete mode 100644 app/views/changeset/list_user.rxml diff --git a/app/controllers/changeset_controller.rb b/app/controllers/changeset_controller.rb index 8d029aae5..3af1f3ab9 100644 --- a/app/controllers/changeset_controller.rb +++ b/app/controllers/changeset_controller.rb @@ -256,77 +256,50 @@ class ChangesetController < ApplicationController def list conditions = conditions_nonempty - # @changesets = Changeset.find(:all, :order => "closed_at DESC", :conditions => ['closed_at < ?', DateTime.now], :limit=> 20) - - #@edit_pages, @edits = paginate(:changesets, - # :include => [:user, :changeset_tags], - # :conditions => conditions, - # :order => "changesets.created_at DESC", - # :per_page => 20) - # - - @edits = Changeset.find(:all, - :order => "changesets.created_at DESC", - :conditions => conditions, - :limit => 20) - end - - ## - # list edits (changesets) belonging to a user - def list_user - user = User.find_by_display_name(params[:display_name], :conditions => {:visible => true}) - - if user - @display_name = user.display_name - if not user.data_public? and @user != user - @edits = nil - render - else - conditions = cond_merge conditions, ['user_id = ?', user.id] - conditions = cond_merge conditions, conditions_nonempty - @edit_pages, @edits = paginate(:changesets, - :include => [:user, :changeset_tags], - :conditions => conditions, - :order => "changesets.created_at DESC", - :per_page => 20) + if params[:display_name] + user = User.find_by_display_name(params[:display_name], :conditions => { :visible => true }) + + if user + if user.data_public? or user == @user + conditions = cond_merge conditions, ['user_id = ?', user.id] + else + conditions = cond_merge conditions, ['false'] + end + elsif params[:format] == 'rhtml' + @not_found_user = params[:display_name] + render :template => 'user/no_such_user', :status => :not_found end - else - @not_found_user = params[:display_name] - render :template => 'user/no_such_user', :status => :not_found end - end - - ## - # list changesets in a bbox - def list_bbox - # support 'bbox' param or alternatively 'minlon', 'minlat' etc - if params['bbox'] - bbox = params['bbox'] - elsif params['minlon'] and params['minlat'] and params['maxlon'] and params['maxlat'] - bbox = h(params['minlon']) + ',' + h(params['minlat']) + ',' + h(params['maxlon']) + ',' + h(params['maxlat']) - elsif params['format'] == "rxml" - raise OSM::APIBadUserInput.new("Bounding box must be supplied for the RSS feed") + + if params[:bbox] + bbox = params[:bbox] + elsif params[:minlon] and params[:minlat] and params[:maxlon] and params[:maxlat] + bbox = params[:minlon] + ',' + params[:minlat] + ',' + params[:maxlon] + ',' + params[:maxlat] + end + + if bbox + conditions = cond_merge conditions, conditions_bbox(bbox) + bbox = BoundingBox.from_s(bbox) + bbox_link = "#{bbox.to_s}" + end + + if user and bbox + @description = t 'changeset.list.description_user_bbox', :user => user.display_name, :bbox => bbox_link + elsif user + @description = t 'changeset.list.description_user', :user => user.display_name + elsif bbox + @description = t 'changeset.list.description_bbox', :bbox => bbox_link else - #TODO: fix bugs in location determination for history tab (and other tabs) then uncomment this redirect - #redirect_to :action => 'list' - - # For now just render immediately, and skip the db - render - return + @description = t 'changeset.list.description' end - - conditions = conditions_bbox(bbox); - conditions = cond_merge conditions, conditions_nonempty - + @edit_pages, @edits = paginate(:changesets, :include => [:user, :changeset_tags], :conditions => conditions, :order => "changesets.created_at DESC", :per_page => 20) - - @bbox = sanitise_boundaries(bbox.split(/,/)) unless bbox==nil end - + private #------------------------------------------------------------ # utility functions below. diff --git a/app/views/changeset/_changeset_paging_nav.rhtml b/app/views/changeset/_changeset_paging_nav.rhtml index b8ac1a65f..94edfc31e 100644 --- a/app/views/changeset/_changeset_paging_nav.rhtml +++ b/app/views/changeset/_changeset_paging_nav.rhtml @@ -1,3 +1,4 @@ +

<% current_page = @edit_pages.current_page %> <%= t'changeset.changeset_paging_nav.showing_page' %> @@ -16,3 +17,4 @@ if @edit_pages.page_count > 1 <% end %> +

diff --git a/app/views/changeset/list.rhtml b/app/views/changeset/list.rhtml index affe10cc6..fba45be50 100644 --- a/app/views/changeset/list.rhtml +++ b/app/views/changeset/list.rhtml @@ -1,13 +1,12 @@ -

<%= t'changeset.list.recent_changes' %>

-

<%= t'changeset.list.recently_edited_changesets' %>

+

<%= t'changeset.list.title' %>

+

<%= @description %>

-<%= render :partial => 'changesets' %> +<%= render :partial => 'changeset_paging_nav' %> +<%= render :partial => 'changesets', :locals => { :showusername => !params.has_key?(:display_name) } %> +<%= render :partial => 'changeset_paging_nav' %> -

<%= t'changeset.list.for_more_changesets' %>

-
- -<%= rss_link_to :action => 'list_rss' %> +<%= rss_link_to params.merge({ :format => 'rxml' }) %> <% content_for :head do %> -<%= auto_discovery_link_tag :atom, :controller => 'changeset', :action => 'list_rss' %> +<%= auto_discovery_link_tag :atom, params.merge({ :format => 'rxml' }) %> <% end %> diff --git a/app/views/changeset/list.rxml b/app/views/changeset/list.rxml index 95e10e8fb..f97516e61 100644 --- a/app/views/changeset/list.rxml +++ b/app/views/changeset/list.rxml @@ -2,18 +2,17 @@ xml.rss("version" => "2.0", "xmlns:geo" => "http://www.w3.org/2003/01/geo/wgs84_pos#", "xmlns:georss" => "http://www.georss.org/georss") do xml.channel do - xml.title t('changeset.list_rss.title') - xml.description t('changeset.list_rss.description') - xml.link url_for(:controller => "browse", :action => "changesets", :only_path => false) + xml.title t('changeset.list.title') + xml.description @description + xml.link url_for(params.merge({ :only_path => false })) xml.image do xml.url "http://www.openstreetmap.org/images/mag_map-rss2.0.png" xml.title "OpenStreetMap" xml.width "100" xml.height "100" - xml.link url_for(:controller => "browse", :action => "changesets", :only_path => false) + xml.link url_for(params.merge({ :only_path => false })) end - for changeset in @edits xml.item do xml.title t('browse.changeset.title') + " " + h(changeset.id) diff --git a/app/views/changeset/list_bbox.rhtml b/app/views/changeset/list_bbox.rhtml deleted file mode 100644 index ef774e378..000000000 --- a/app/views/changeset/list_bbox.rhtml +++ /dev/null @@ -1,51 +0,0 @@ -

<%= t'changeset.list_bbox.history' %>

-<% -if @bbox!=nil - minlon = @bbox[0] - minlat = @bbox[1] - maxlon = @bbox[2] - maxlat = @bbox[3] - - %> -

-<%= t'changeset.list_bbox.changesets_within_the_area' %> - ('><%= format("%0.3f",minlon) -%>,<%= format("%0.3f",minlat) -%>,<%= format("%0.3f",maxlon) -%>,<%= format("%0.3f",maxlat) -%>) - -

- -<% if @edits.nil? or @edits.empty? %> -

<%= t'changeset.list_bbox.no_changesets' %>

-<% else %> - -<%= render :partial => 'changeset_paging_nav' %> -<%= render :partial => 'changesets' %> -<%= render :partial => 'changeset_paging_nav' %> - -

<%= t'changeset.list_bbox.all_changes_everywhere' , :recent_changes_link => link_to(t('changeset.list_bbox.recent_changes'), :controller => "browse", :action => "changesets") %>

- -<% - end - -else - #bbox is nil. happens if the user surfs to this page directly. -%> - -

<%= t'changeset.list_bbox.no_area_specified' %>

-

<%= t'changeset.list_bbox.first_use_view', :view_tab_link => '' + t('changeset.list_bbox.view_tab') + '' %>

-

<%= t'changeset.list_bbox.alternatively_view', :recent_changes_link => link_to(t('changeset.list_bbox.recent_changes'), :controller => "browse", :action => "changesets") %>

- -<% -end -%> -
- -<% if @bbox %> - <%= rss_link_to :action => 'list_bbox_rss', :bbox => @bbox.join(",") %> -<% end %> - - -<% if @bbox %> - <% content_for :head do %> - <%= auto_discovery_link_tag :atom, :controller => 'changeset', :action => 'list_bbox_rss', :bbox => @bbox.join(",") %> - <% end %> -<% end %> diff --git a/app/views/changeset/list_bbox.rxml b/app/views/changeset/list_bbox.rxml deleted file mode 100644 index 172523cba..000000000 --- a/app/views/changeset/list_bbox.rxml +++ /dev/null @@ -1,48 +0,0 @@ -xml.rss("version" => "2.0", - "xmlns:geo" => "http://www.w3.org/2003/01/geo/wgs84_pos#", - "xmlns:georss" => "http://www.georss.org/georss") do - xml.channel do - xml.title t('changeset.list_bbox_rss.title', :bbox => @bbox.join(",")) - xml.description t('changeset.list_bbox_rss.description', :bbox => @bbox.join(",")) - xml.link url_for(:controller => "history", :bbox => @bbox.join(","), :only_path => false) - xml.image do - xml.url "http://www.openstreetmap.org/images/mag_map-rss2.0.png" - xml.title "OpenStreetMap" - xml.width "100" - xml.height "100" - xml.link url_for(:controller => "history", :bbox => @bbox.join(","), :only_path => false) - end - - - for changeset in @edits - xml.item do - xml.title t('browse.changeset.title') + " " + h(changeset.id) - xml.link url_for(:controller => 'browse', :action => "changeset", :id => changeset.id, :only_path => false) - xml.guid url_for(:controller => 'browse', :action => "changeset", :id => changeset.id, :only_path => false) - if changeset.user.data_public? - xml.author changeset.user.display_name - end - if changeset.tags['comment'] - xml.description changeset.tags['comment'] - end - xml.pubDate changeset.created_at.to_s(:rfc822) - if changeset.user.data_public? - xml.comments url_for(:controller => "message", :action => "new", :id => changeset.user.id, :only_path => false) - end - - unless changeset.min_lat.nil? - minlon = changeset.min_lon/GeoRecord::SCALE.to_f - minlat = changeset.min_lat/GeoRecord::SCALE.to_f - maxlon = changeset.max_lon/GeoRecord::SCALE.to_f - maxlat = changeset.max_lat/GeoRecord::SCALE.to_f - - # See http://georss.org/Encodings#Geometry - lower_corner = "#{minlat} #{minlon}" - upper_corner = "#{maxlat} #{maxlon}" - - xml.georss :box, lower_corner + " " + upper_corner - end - end - end - end -end diff --git a/app/views/changeset/list_user.rhtml b/app/views/changeset/list_user.rhtml deleted file mode 100644 index 6f246f9d4..000000000 --- a/app/views/changeset/list_user.rhtml +++ /dev/null @@ -1,18 +0,0 @@ -

<%= t'changeset.list_user.edits_by_username', :username_link => link_to(h(@display_name), {:controller=>'user', :action=>'view', :display_name=>@display_name}) %>

- -<% if not @edits or @edits.empty? %> -

<%= t'changeset.list_user.no_visible_edits_by', :name => h(@display_name) %>.

-<% else %> -<%= render :partial => 'changeset_paging_nav' %> -<%= render :partial => 'changesets', :locals => {:showusername => false} %> -<%= render :partial => 'changeset_paging_nav' %> -<% end %> - -

<%= t'changeset.list_user.for_all_changes', :recent_changes_link => link_to(t('changeset.list_user.recent_changes'), :controller => "browse", :action => "changesets") %>

-
- -<%= rss_link_to :action => 'list_user_rss' %> - -<% content_for :head do %> -<%= auto_discovery_link_tag :atom, :controller => 'changeset', :action => 'list_user_rss' %> -<% end %> diff --git a/app/views/changeset/list_user.rxml b/app/views/changeset/list_user.rxml deleted file mode 100644 index ed6c6459d..000000000 --- a/app/views/changeset/list_user.rxml +++ /dev/null @@ -1,48 +0,0 @@ -xml.rss("version" => "2.0", - "xmlns:geo" => "http://www.w3.org/2003/01/geo/wgs84_pos#", - "xmlns:georss" => "http://www.georss.org/georss") do - xml.channel do - xml.title t('changeset.list_user_rss.title', :user => @display_name) - xml.description t('changeset.list_user_rss.description', :user => @display_name) - xml.link url_for(:controller => "user", :action => "edits", :id => @display_name, :only_path => false) - xml.image do - xml.url "http://www.openstreetmap.org/images/mag_map-rss2.0.png" - xml.title "OpenStreetMap" - xml.width "100" - xml.height "100" - xml.link url_for(:controller => "user", :action => "edits", :id => @display_name, :only_path => false) - end - - - for changeset in @edits - xml.item do - xml.title t('browse.changeset.title') + " " + h(changeset.id) - xml.link url_for(:controller => 'browse', :action => "changeset", :id => changeset.id, :only_path => false) - xml.guid url_for(:controller => 'browse', :action => "changeset", :id => changeset.id, :only_path => false) - if changeset.user.data_public? - xml.author changeset.user.display_name - end - if changeset.tags['comment'] - xml.description changeset.tags['comment'] - end - xml.pubDate changeset.created_at.to_s(:rfc822) - if changeset.user.data_public? - xml.comments url_for(:controller => "message", :action => "new", :id => changeset.user.id, :only_path => false) - end - - unless changeset.min_lat.nil? - minlon = changeset.min_lon/GeoRecord::SCALE.to_f - minlat = changeset.min_lat/GeoRecord::SCALE.to_f - maxlon = changeset.max_lon/GeoRecord::SCALE.to_f - maxlat = changeset.max_lat/GeoRecord::SCALE.to_f - - # See http://georss.org/Encodings#Geometry - lower_corner = "#{minlat} #{minlon}" - upper_corner = "#{maxlat} #{maxlon}" - - xml.georss :box, lower_corner + " " + upper_corner - end - end - end - end -end diff --git a/config/locales/en.yml b/config/locales/en.yml index a4b0f352c..d46245ad1 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -217,36 +217,12 @@ en: user: "User" comment: "Comment" area: "Area" - list_bbox: - history: "History" - changesets_within_the_area: "Changesets within the area:" - show_area_box: "show area box" - no_changesets: "No changesets" - all_changes_everywhere: "For all changes everywhere see {{recent_changes_link}}" - recent_changes: "Recent Changes" - no_area_specified: "No area specified" - first_use_view: "First use the {{view_tab_link}} to pan and zoom to an area of interest, then click the history tab." - view_the_map: "view the map" - view_tab: "view tab" - alternatively_view: "Alternatively, view all {{recent_changes_link}}" list: - recent_changes: "Recent Changes" - recently_edited_changesets: "Recently edited changesets:" - for_more_changesets: "For more changesets, select a user and view their edits, or see the editing 'history' of a specific area." - list_rss: - title: "Recent changes" - description: "Recently edited changesets" - list_bbox_rss: - title: "Recent changesets within {{bbox}}" - description: "Recently edited changesets within the area {{bbox}}" - list_user: - edits_by_username: "Edits by {{username_link}}" - no_visible_edits_by: "No visible edits by {{name}}." - for_all_changes: "For changes by all users see {{recent_changes_link}}" - recent_changes: "Recent Changes" - list_user_rss: - title: "Edits by {{user}}" - description: "Recent changesets by {{user}}" + title: "Changesets" + description: "Recent edits" + description_user: "Recent edits by {{user}}" + description_bbox: "Recent edits within {{bbox}}" + description_user_bbox: "Recent edits by {{user}} within {{bbox}}" diary_entry: new: title: New Diary Entry diff --git a/config/routes.rb b/config/routes.rb index 1b5d483c6..68f1ad6d2 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -73,7 +73,7 @@ ActionController::Routing::Routes.draw do |map| map.connect "api/#{API_VERSION}/swf/trackpoints", :controller =>'swf', :action =>'trackpoints' # Data browsing - map.connect '/browse', :controller => 'changeset', :action => 'list' + map.connect '/browse', :controller => 'changeset', :action => 'list', :format => 'rhtml' map.connect '/browse/start', :controller => 'browse', :action => 'start' map.connect '/browse/way/:id', :controller => 'browse', :action => 'way', :id => /\d+/ map.connect '/browse/way/:id/history', :controller => 'browse', :action => 'way_history', :id => /\d+/ @@ -89,8 +89,8 @@ ActionController::Routing::Routes.draw do |map| map.root :controller => 'site', :action => 'index' map.connect '/', :controller => 'site', :action => 'index' map.connect '/edit', :controller => 'site', :action => 'edit' - map.connect '/history', :controller => 'changeset', :action => 'list_bbox', :format => 'rhtml' - map.connect '/history/rss', :controller => 'changeset', :action => 'list_bbox', :format => 'rxml' + map.connect '/history', :controller => 'changeset', :action => 'list', :format => 'rhtml' + map.connect '/history/rss', :controller => 'changeset', :action => 'list', :format => 'rxml' map.connect '/export', :controller => 'site', :action => 'export' map.connect '/login', :controller => 'user', :action => 'login' map.connect '/logout', :controller => 'user', :action => 'logout' @@ -144,8 +144,8 @@ ActionController::Routing::Routes.draw do |map| # user pages map.connect '/user/:display_name', :controller => 'user', :action => 'view' - map.connect '/user/:display_name/edits', :controller => 'changeset', :action => 'list_user', :format => 'rhtml' - map.connect '/user/:display_name/edits/rss', :controller => 'changeset', :action => 'list_user', :format => 'rxml' + map.connect '/user/:display_name/edits', :controller => 'changeset', :action => 'list', :format => 'rhtml' + map.connect '/user/:display_name/edits/rss', :controller => 'changeset', :action => 'list', :format => 'rxml' map.connect '/user/:display_name/make_friend', :controller => 'user', :action => 'make_friend' map.connect '/user/:display_name/remove_friend', :controller => 'user', :action => 'remove_friend' map.connect '/user/:display_name/diary', :controller => 'diary_entry', :action => 'list' diff --git a/test/functional/changeset_controller_test.rb b/test/functional/changeset_controller_test.rb index d60fe2fbc..f1ecb3cc7 100644 --- a/test/functional/changeset_controller_test.rb +++ b/test/functional/changeset_controller_test.rb @@ -1490,11 +1490,11 @@ EOF def test_list changesets = Changeset.find(:all, :order => "created_at DESC", :conditions => ['min_lat IS NOT NULL'], :limit=> 20) assert changesets.size <= 20 - get :list + get :list, {:format => "rhtml"} assert_response :success assert_template "list" # Now check that all 20 (or however many were returned) changesets are in the html - assert_select "h1", :text => "Recent Changes", :count => 1 + assert_select "h1", :text => "Changesets", :count => 1 assert_select "table[id='changeset_list'] tr", :count => changesets.size + 1 changesets.each do |changeset| # FIXME this test needs rewriting - test for table contents @@ -1505,16 +1505,16 @@ EOF # Checks the display of the user changesets listing def test_list_user user = users(:public_user) - get :list_user, {:display_name => user.display_name} + get :list, {:format => "rhtml", :display_name => user.display_name} assert_response :success - assert_template 'list_user' + assert_template "list" ## FIXME need to add more checks to see which if edits are actually shown if your data is public end ## # Check the not found of the list user changesets def test_list_user_not_found - get :list_user, {:display_name => "Some random user"} + get :list, {:format => "rhtml", :display_name => "Some random user"} assert_response :not_found assert_template 'user/no_such_user' end -- 2.43.2