Tidy up trace handling a bit, and add support for per-user and per-tag feeds.

app/controllers/trace_controller.rb

@@ -5,28 +5,24 @@ class TraceController < ApplicationController
  
   # Counts and selects pages of GPX traces for various criteria (by user, tags, public etc.).
   #  target_user - if set, specifies the user to fetch traces for.  if not set will fetch all traces
-  def list (target_user = nil)
+  def list(target_user = nil, action = "list")
     # from display name, pick up user id if one user's traces only
     display_name = params[:display_name]
     if target_user.nil? and !display_name.blank?
-      @display_name = display_name
       target_user = User.find(:first, :conditions => [ "display_name = ?", display_name])
     end
 
     # set title
     if target_user.nil?
-      @title = "public GPS traces"
-    elsif target_user.id == @user.id
-      @title = "your GPS traces"
+      @title = "Public GPS traces"
+    elsif @user and @user.id == target_user.id
+      @title = "Your GPS traces"
     else
-      @title = "public GPS traces from #{target_user.display_name}"
+      @title = "Public GPS traces from #{target_user.display_name}"
     end
 
     @title += " tagged with #{params[:tag]}" if params[:tag]
 
-    opt = Hash.new
-    opt[:include] = [:user, :tags] # load users and tags from db at same time as traces
-
     # four main cases:
     # 1 - all traces, logged in = all public traces + all user's (i.e + all mine)
     # 2 - all traces, not logged in = all public traces
@@ -34,31 +30,30 @@ class TraceController < ApplicationController
     # 4 - user's traces, not logged in as that user = all user's public traces
     if target_user.nil? # all traces
       if @user
-        conditions = ["(public = 1 OR user_id = ?)", @user.id] #1
+        conditions = ["(gpx_files.public = 1 OR gpx_files.user_id = ?)", @user.id] #1
       else
-        conditions  = ["public = 1"] #2
+        conditions  = ["gpx_files.public = 1"] #2
       end
     else
       if @user and @user.id == target_user.id
-        conditions = ["user_id = ?", @user.id] #3 (check vs user id, so no join + can't pick up non-public traces by changing name)
+        conditions = ["gpx_files.user_id = ?", @user.id] #3 (check vs user id, so no join + can't pick up non-public traces by changing name)
       else
-        conditions = ["public = 1 AND user_id = ?", target_user.id] #4
+        conditions = ["gpx_files.public = 1 AND gpx_files.user_id = ?", target_user.id] #4
       end
     end
-    conditions[0] += " AND users.display_name != ''" # users need to set display name before traces will be exposed
     
-    opt[:order] = 'timestamp DESC'
     if params[:tag]
       @tag = params[:tag]
-      conditions[0] += " AND gpx_file_tags.tag = ?"
-      conditions << @tag;
+      conditions[0] += " AND EXISTS (SELECT * FROM gpx_file_tags AS gft WHERE gft.gpx_id = gpx_files.id AND gft.tag = ?)"
+      conditions << @tag
     end
     
-    opt[:conditions] = conditions
-    opt[:per_page] = 20
+    @trace_pages, @traces = paginate(:traces,
+                                     :include => [:user, :tags],
+                                     :conditions => conditions,
+                                     :order => "gpx_files.timestamp DESC",
+                                     :per_page => 20)
 
-    @trace_pages, @traces = paginate(:traces, opt)
-    
     # put together SET of tags across traces, for related links
     tagset = Hash.new
     if @traces
@@ -71,13 +66,14 @@ class TraceController < ApplicationController
     end
     
     # final helper vars for view
-    @display_name = display_name
+    @action = action
+    @display_name = target_user.display_name if target_user
     @all_tags = tagset.values
   end
 
   def mine
     if @user
-      list(@user) unless @user.nil?
+      list(@user, "mine") unless @user.nil?
     else
       redirect_to :controller => 'user', :action => 'login', :referer => request.request_uri
     end
@@ -85,6 +81,7 @@ class TraceController < ApplicationController
 
   def view
     @trace = Trace.find(params[:id])
+    @title = "Viewing trace #{@trace.name}"
     unless @trace.public
       if @user
         render :nothing, :status => :forbidden if @trace.user.id != @user.id
@@ -129,11 +126,23 @@ class TraceController < ApplicationController
   end
 
   def georss
-    traces = Trace.find(:all, :conditions => ['public = true'], :order => 'timestamp DESC', :limit => 20)
+    conditions = ["gpx_files.public = 1"]
+
+    if params[:display_name]
+      conditions[0] += " AND users.display_name = ?"
+      conditions << params[:display_name]
+    end
+    
+    if params[:tag]
+      conditions[0] += " AND EXISTS (SELECT * FROM gpx_file_tags AS gft WHERE gft.gpx_id = gpx_files.id AND gft.tag = ?)"
+      conditions << params[:tag]
+    end
+
+    traces = Trace.find(:all, :include => :user, :conditions => conditions, 
+                        :order => "timestamp DESC", :limit => 20)
 
     rss = OSM::GeoRSS.new
 
-    #def add(latitude=0, longitude=0, title_text='dummy title', url='http://www.example.com/', description_text='dummy description', timestamp=Time.now)
     traces.each do |trace|
       rss.add(trace.latitude, trace.longitude, trace.name, trace.user.display_name, url_for({:controller => 'trace', :action => 'view', :id => trace.id, :display_name => trace.user.display_name}), "<img src='#{url_for({:controller => 'trace', :action => 'icon', :id => trace.id, :user_login => trace.user.display_name})}'> GPX file with #{trace.size} points from #{trace.user.display_name}", trace.timestamp)
     end

app/helpers/trace_helper.rb

@@ -1,2 +1,9 @@
 module TraceHelper
+  def link_to_tag(tag)
+    if @action == "mine"
+      return link_to tag, :tag => tag
+    else
+      return link_to tag, :tag => tag, :display_name => @display_name
+    end
+  end
 end

app/views/trace/_trace.rhtml

@@ -14,8 +14,8 @@
       <% end %> 
       ... <%= time_ago_in_words( trace.timestamp ) %>  ago</span>
       <%= link_to 'more', {:controller => 'trace', :action => 'view', :display_name => trace.user.display_name, :id => trace.id}, {:title => 'View Trace Details'} %> /
-      <%= link_to 'map', {:controller => 'site', :action => 'index', :lat => trace.latitude, :lon => trace.longitude, :zoom => 14}, {:title => 'View Map'} %> /
-      <%= link_to 'edit', {:controller => 'site', :action => 'edit', :lat => trace.latitude, :lon => trace.longitude, :zoom => 14}, {:title => 'Edit Map'} %>
+      <%= link_to_if trace.inserted?, 'map', {:controller => 'site', :action => 'index', :lat => trace.latitude, :lon => trace.longitude, :zoom => 14}, {:title => 'View Map'} %> /
+      <%= link_to_if trace.inserted?, 'edit', {:controller => 'site', :action => 'edit', :lat => trace.latitude, :lon => trace.longitude, :zoom => 14}, {:title => 'Edit Map'} %>
       <br />
       <%= escape_once(trace.description) %>
     <br />
@@ -23,7 +23,7 @@
     in 
     <% if trace.tags %>
       <% trace.tags.each do |tag| %>
-        <%= link_to tag.tag, :tag => tag.tag %>
+        <%= link_to_tag tag.tag %>
       <% end %>
     <% end %>
   </td>

app/views/trace/_trace_header.rhtml

@@ -0,0 +1,15 @@
+<h1><%= @title %></h1>
+
+<span class="rsssmall"><a href="<%= url_for :action => 'georss', :display_name => @display_name, :tag => @tag %>"><img src="/images/RSS.gif" border="0" alt="RSS" /></a></span>
+<% if @user.nil? or @display_name.nil? or @user.display_name != @display_name %>
+ | <%= link_to 'See just your traces, or upload a trace', :action => 'mine' %>
+<% end %>
+<% if @tag or @display_name %>
+ | <%= link_to 'See all traces', :controller => 'trace', :action => 'list' %>
+<% end %>
+<% if @tag and @user and @user.display_name == @display_name %>
+ | <%= link_to 'See all your traces', :controller => 'trace', :action => 'mine' %>
+<% end %>
+
+<br />
+<br />

app/views/trace/_trace_optionals.rhtml

@@ -5,7 +5,7 @@
     <br />
     <% if @all_tags %>
       <% @all_tags.each do |tag| %>
-        <%= link_to tag, :tag => tag %><br />
+        <%= link_to_tag tag %><br />
       <% end %>
     <% end %>
   </div>

app/views/trace/list.rhtml

@@ -1,16 +1,3 @@
-<h1>Public GPS Traces</h1>
-
-<% if @tag %>
- Traces filtered by tag <b><%= @tag %></b>
- <br/><br/>
-<% end %>
-<span class="rsssmall"><a href="<%= url_for :controller => 'trace', :action => 'georss' %>"><img src="/images/RSS.gif" border="0" alt="RSS" /></a></span> | 
-<%= link_to 'See just your traces, or upload a trace', {:controller => 'trace', :action => 'mine'} %>
-<% if @tag %>
- | <%= link_to 'See all traces', {:controller => 'trace', :action => 'list'} %>
-<% end %>
-
-<br />
-<br />
+<%= render :partial => 'trace_header' %>
 
 <%= render :partial => 'trace_list' %>

app/views/trace/mine.rhtml

@@ -1,18 +1,4 @@
-<h1>Your GPS Traces</h1>
-
-<% if @tag %>
- Traces filtered by tag <b><%= @tag %></b>
- <br/><br/>
-<% end %>
-<%= link_to 'See all traces', {:controller => 'trace', :action => 'list'} %>
-<% if @tag %>
-  | <%= link_to 'See all your traces', {:controller => 'trace', :action => 'mine'} %>
-<% end %>
-
-<br />
-<br />
-
-<% if @user %>
+<%= render :partial => 'trace_header' %>
 
 <% form_tag({:action => 'create'}, :multipart => true) do %>
 <table>
@@ -28,5 +14,3 @@
 <% end %>
 
 <%= render :partial => 'trace_list' %>
-
-<% end %>

app/views/trace/view.rhtml

@@ -1,12 +1,14 @@
-<h2>Viewing trace <%= @trace.name %></h2>
+<h2><%= @title %></h2>
 
 <img src="<%= url_for :controller => 'trace', :action => 'picture', :id => @trace.id, :user_login => @trace.user.display_name %>">
 
 <table border="0">
   <tr><td>filename:</td><td><%= @trace.name %> (<%= link_to 'download', :controller => 'trace', :action => 'data', :id => @trace.id %>)</td></tr> <!-- TODO link to download -->
   <tr><td>uploaded at:</td><td><%= @trace.timestamp %></td></tr>
-  <tr><td>points:</td><td><%= @trace.size.to_s.gsub(/(\d)(?=(\d{3})+$)/,'\1,') %></td></tr>
-  <tr><td>start coordinate:</td><td><%= @trace.latitude %>, <%= @trace.longitude %> (<%=link_to 'map', :controller => 'site', :action => 'index', :lat => @trace.latitude, :lon => @trace.longitude, :zoom => 14 %> / <%=link_to 'edit', :controller => 'site', :action => 'edit', :lat => @trace.latitude, :lon => @trace.longitude, :zoom => 14 %>)</td></tr>
+  <% if @trace.inserted? %>
+    <tr><td>points:</td><td><%= @trace.size.to_s.gsub(/(\d)(?=(\d{3})+$)/,'\1,') %></td></tr>
+    <tr><td>start coordinate:</td><td><%= @trace.latitude %>, <%= @trace.longitude %> (<%=link_to 'map', :controller => 'site', :action => 'index', :lat => @trace.latitude, :lon => @trace.longitude, :zoom => 14 %> / <%=link_to 'edit', :controller => 'site', :action => 'edit', :lat => @trace.latitude, :lon => @trace.longitude, :zoom => 14 %>)</td></tr>
+  <% end %>
   <tr><td>owner:</td><td><%= link_to @trace.user.display_name, {:controller => 'trace', :action => 'view', :display_name => @trace.user.display_name, :id => nil} %></td></tr>
   <tr><td>description:</td><td><%= @trace.description %></td></tr>
   <tr><td>tags:</td><td>

config/routes.rb

@@ -64,19 +64,24 @@ ActionController::Routing::Routes.draw do |map|
   # traces  
   map.connect '/traces', :controller => 'trace', :action => 'list'
   map.connect '/traces/page/:page', :controller => 'trace', :action => 'list'
+  map.connect '/traces/rss', :controller => 'trace', :action => 'georss'
+  map.connect '/traces/tag/:tag', :controller => 'trace', :action => 'list'
+  map.connect '/traces/tag/:tag/page/:page', :controller => 'trace', :action => 'list'
+  map.connect '/traces/tag/:tag/rss', :controller => 'trace', :action => 'georss'
   map.connect '/traces/mine', :controller => 'trace', :action => 'mine'
-  map.connect '/trace/create', :controller => 'trace', :action => 'create'
   map.connect '/traces/mine/page/:page', :controller => 'trace', :action => 'mine'
   map.connect '/traces/mine/tag/:tag', :controller => 'trace', :action => 'mine'
   map.connect '/traces/mine/tag/:tag/page/:page', :controller => 'trace', :action => 'mine'
-  map.connect '/traces/rss', :controller => 'trace', :action => 'georss'
-  map.connect '/user/:display_name/traces', :controller => 'trace', :action => 'list', :id => nil
-  map.connect '/user/:display_name/traces/page/:page', :controller => 'trace', :action => 'list', :id => nil
-  map.connect '/user/:display_name/traces/:id', :controller => 'trace', :action => 'view', :id => nil
-  map.connect '/user/:display_name/traces/:id/picture', :controller => 'trace', :action => 'picture', :id => nil
-  map.connect '/user/:display_name/traces/:id/icon', :controller => 'trace', :action => 'icon', :id => nil
-  map.connect '/traces/tag/:tag', :controller => 'trace', :action => 'list', :id => nil
-  map.connect '/traces/tag/:tag/page/:page', :controller => 'trace', :action => 'list', :id => nil
+  map.connect '/trace/create', :controller => 'trace', :action => 'create'
+  map.connect '/user/:display_name/traces', :controller => 'trace', :action => 'list'
+  map.connect '/user/:display_name/traces/page/:page', :controller => 'trace', :action => 'list'
+  map.connect '/user/:display_name/traces/rss', :controller => 'trace', :action => 'georss'
+  map.connect '/user/:display_name/traces/tag/:tag', :controller => 'trace', :action => 'list'
+  map.connect '/user/:display_name/traces/tag/:tag/page/:page', :controller => 'trace', :action => 'list'
+  map.connect '/user/:display_name/traces/tag/:tag/rss', :controller => 'trace', :action => 'georss'
+  map.connect '/user/:display_name/traces/:id', :controller => 'trace', :action => 'view'
+  map.connect '/user/:display_name/traces/:id/picture', :controller => 'trace', :action => 'picture'
+  map.connect '/user/:display_name/traces/:id/icon', :controller => 'trace', :action => 'icon'
 
   # user pages
   map.connect '/user/:display_name/make_friend', :controller => 'user', :action => 'make_friend'