From ba0b52ff1aa0a56ada5939b315e84c972cb2ec34 Mon Sep 17 00:00:00 2001 From: lukemelia Date: Mon, 2 Apr 2007 04:18:19 +0000 Subject: [PATCH] Merged mobile_controller into the todos_controller. The lightweight mobile HTML is arguably just another representation of the same resources, so it seems to fit the RESTful Rails paradigm to use an extension (.m) to switch on in the respond_to stanza. I needed some hackery to make this work. See my note in todos_controller for a full explanation. I also added a route to get to the mobile view by using 'domain.com/m' Created some selenium tests for the mobile view, too. In optimizing the data access for the mobile view, I ran into "a bug in rails pagination":http://dev.rubyonrails.org/ticket/7885" and integrated a nice pagination plugin from the Err the Blog guys ("will_paginate":http://errtheblog.com/post/929) to work around the issue. NOTE that this changeset includes a new line in environment.rb.tmpl (at the bottom). Be sure to copy this into your environment.rb file. These changes fix #489 (cannot edit action using mobile interface). Thanks for the bug report, lrbalt! In the name of consistency, I made the argument to the block for all respond_to calls 'format' (instead of the formerly cool 'wants'). Lastly, I added a link to the project's new contribute page to the footer of the main web UI. Help us join the Mac on Intel world. :-) git-svn-id: http://www.rousette.org.uk/svn/tracks-repos/trunk@517 a4c988fc-2ded-0310-b66e-134b36920a42 --- tracks/app/controllers/application.rb | 10 +- tracks/app/controllers/contexts_controller.rb | 6 +- tracks/app/controllers/mobile_controller.rb | 89 --------- tracks/app/controllers/notes_controller.rb | 6 +- tracks/app/controllers/projects_controller.rb | 6 +- tracks/app/controllers/todos_controller.rb | 174 ++++++++++++++---- tracks/app/helpers/todos_helper.rb | 5 + tracks/app/views/mobile/_mobile_actions.rhtml | 35 ---- tracks/app/views/mobile/detail.rhtml | 4 - tracks/app/views/mobile/filter.rhtml | 6 - tracks/app/views/mobile/index.rhtml | 5 - tracks/app/views/mobile/show_add_form.rhtml | 4 - tracks/app/views/shared/_footer.rhtml | 2 +- .../_edit_mobile.rhtml} | 5 +- tracks/app/views/todos/_mobile_actions.rhtml | 33 ++++ tracks/app/views/todos/index_mobile.rhtml | 6 + tracks/app/views/todos/new_mobile.rhtml | 5 + tracks/app/views/todos/show_mobile.rhtml | 5 + tracks/config/environment.rb.tmpl | 3 + tracks/config/routes.rb | 19 +- tracks/db/schema.rb | 2 +- tracks/lib/login_system.rb | 1 + tracks/lib/source_view.rb | 2 +- .../test/functional/mobile_controller_test.rb | 54 ------ .../test/functional/todos_controller_test.rb | 29 +++ .../selenium/mobile/create_new_action.rsel | 18 ++ tracks/test/selenium/mobile/mark_done.rsel | 11 ++ tracks/test/selenium/mobile/navigation.rsel | 25 +++ tracks/vendor/plugins/will_paginate/README | 58 ++++++ tracks/vendor/plugins/will_paginate/init.rb | 4 + .../plugins/will_paginate/lib/finder.rb | 28 +++ .../will_paginate/lib/will_paginate.rb | 43 +++++ 32 files changed, 445 insertions(+), 258 deletions(-) delete mode 100644 tracks/app/controllers/mobile_controller.rb delete mode 100644 tracks/app/views/mobile/_mobile_actions.rhtml delete mode 100644 tracks/app/views/mobile/detail.rhtml delete mode 100644 tracks/app/views/mobile/filter.rhtml delete mode 100644 tracks/app/views/mobile/index.rhtml delete mode 100644 tracks/app/views/mobile/show_add_form.rhtml rename tracks/app/views/{mobile/_mobile_edit.rhtml => todos/_edit_mobile.rhtml} (87%) create mode 100644 tracks/app/views/todos/_mobile_actions.rhtml create mode 100644 tracks/app/views/todos/index_mobile.rhtml create mode 100644 tracks/app/views/todos/new_mobile.rhtml create mode 100644 tracks/app/views/todos/show_mobile.rhtml delete mode 100644 tracks/test/functional/mobile_controller_test.rb create mode 100644 tracks/test/selenium/mobile/create_new_action.rsel create mode 100644 tracks/test/selenium/mobile/mark_done.rsel create mode 100644 tracks/test/selenium/mobile/navigation.rsel create mode 100644 tracks/vendor/plugins/will_paginate/README create mode 100644 tracks/vendor/plugins/will_paginate/init.rb create mode 100644 tracks/vendor/plugins/will_paginate/lib/finder.rb create mode 100644 tracks/vendor/plugins/will_paginate/lib/will_paginate.rb diff --git a/tracks/app/controllers/application.rb b/tracks/app/controllers/application.rb index 52d4b533..bc4c1610 100644 --- a/tracks/app/controllers/application.rb +++ b/tracks/app/controllers/application.rb @@ -28,7 +28,7 @@ class ApplicationController < ActionController::Base def set_charset headers["Content-Type"] ||= "text/html; charset=UTF-8" end - + def set_session_expiration # http://wiki.rubyonrails.com/rails/show/HowtoChangeSessionOptions unless session == nil @@ -56,13 +56,13 @@ class ApplicationController < ActionController::Base def rescue_action(exception) log_error(exception) if logger - respond_to do |wants| - wants.html do + respond_to do |format| + format.html do notify :warning, "An error occurred on the server." render :action => "index" end - wants.js { render :action => 'error' } - wants.xml { render :text => 'An error occurred on the server.' + $! } + format.js { render :action => 'error' } + format.xml { render :text => 'An error occurred on the server.' + $! } end end diff --git a/tracks/app/controllers/contexts_controller.rb b/tracks/app/controllers/contexts_controller.rb index b92d7107..d18883ed 100644 --- a/tracks/app/controllers/contexts_controller.rb +++ b/tracks/app/controllers/contexts_controller.rb @@ -41,9 +41,9 @@ class ContextsController < ApplicationController end @saved = @context.save @context_not_done_counts = { @context.id => 0 } - respond_to do |wants| - wants.js - wants.xml do + respond_to do |format| + format.js + format.xml do if @context.new_record? && params_are_invalid render_failure "Expected post format is valid xml like so: context name." elsif @context.new_record? diff --git a/tracks/app/controllers/mobile_controller.rb b/tracks/app/controllers/mobile_controller.rb deleted file mode 100644 index 90459347..00000000 --- a/tracks/app/controllers/mobile_controller.rb +++ /dev/null @@ -1,89 +0,0 @@ -class MobileController < ApplicationController - - layout 'mobile' - - before_filter :init, :except => :update - - # Plain list of all next actions, paginated 6 per page - # Sorted by due date, then creation date - # - def index - @page_title = @desc = "All actions" - @todos_pages, @todos = paginate( :todos, :order => 'due IS NULL, due ASC, created_at ASC', - :conditions => ['user_id = ? and state = ?', @user.id, "active"], - :per_page => 6 ) - @count = @all_todos.reject { |x| !x.active? || x.context.hide? }.size - end - - def detail - @item = check_user_return_item - @place = @item.context.id - end - - def update - if params[:id] - @item = check_user_return_item - @item.update_attributes params[:todo] - if params[:todo][:state] == "1" - @item.state = "completed" - else - @item.state = "active" - end - else - params[:todo][:user_id] = @user.id - @item = Todo.new(params[:todo]) if params[:todo] - end - - if @item.save - redirect_to :action => 'index' - else - self.init - if params[:id] - render :partial => 'mobile_edit' - else - render :action => 'show_add_form' - end - end - end - - def show_add_form - # Just render the view - end - - def filter - @type = params[:type] - case params[:type] - when 'context' - @context = Context.find( params[:context][:id] ) - @page_title = @desc = "#{@context.name}" - @todos = Todo.find( :all, :order => 'due IS NULL, due ASC, created_at ASC', - :conditions => ['user_id = ? and state = ? and context_id = ?', @user.id, "active", @context.id] ) - @count = @all_todos.reject { |x| x.completed? || x.context_id != @context.id }.size - when 'project' - @project = Project.find( params[:project][:id] ) - @page_title = @desc = "#{@project.name}" - @todos = Todo.find( :all, :order => 'due IS NULL, due ASC, created_at ASC', - :conditions => ['user_id = ? and state = ? and project_id = ?', @user.id, "active", @project.id] ) - @count = @all_todos.reject { |x| x.completed? || x.project_id != @project.id }.size - end - end - - protected - - def check_user_return_item - item = Todo.find( params['id'] ) - if @user == item.user - return item - else - notify :warning, "Item and session user mis-match: #{item.user.name} and #{@user.name}!" - render_text "" - end - end - - def init - @contexts = @user.contexts.find(:all, :order => 'position ASC') - @projects = @user.projects.find_in_state(:all, :active, :order => 'position ASC') - @all_todos = @user.todos.find(:all, :conditions => ['state = ? or state = ?', "active", "completed"]) - end - -end diff --git a/tracks/app/controllers/notes_controller.rb b/tracks/app/controllers/notes_controller.rb index 03447eb7..c2e7fbdd 100644 --- a/tracks/app/controllers/notes_controller.rb +++ b/tracks/app/controllers/notes_controller.rb @@ -3,9 +3,9 @@ class NotesController < ApplicationController def index @all_notes = @user.notes @page_title = "TRACKS::All notes" - respond_to do |wants| - wants.html - wants.xml { render :xml => @all_notes.to_xml( :except => :user_id ) } + respond_to do |format| + format.html + format.xml { render :xml => @all_notes.to_xml( :except => :user_id ) } end end diff --git a/tracks/app/controllers/projects_controller.rb b/tracks/app/controllers/projects_controller.rb index f9bee11f..cae0a7d2 100644 --- a/tracks/app/controllers/projects_controller.rb +++ b/tracks/app/controllers/projects_controller.rb @@ -53,9 +53,9 @@ class ProjectsController < ApplicationController @project_not_done_counts = { @project.id => 0 } @active_projects_count = @user.projects.count(:conditions => "state = 'active'") @contexts = @user.contexts - respond_to do |wants| - wants.js - wants.xml do + respond_to do |format| + format.js + format.xml do if @project.new_record? && params_are_invalid render_failure "Expected post format is valid xml like so: project name." elsif @project.new_record? diff --git a/tracks/app/controllers/todos_controller.rb b/tracks/app/controllers/todos_controller.rb index 36575c90..2925f76d 100644 --- a/tracks/app/controllers/todos_controller.rb +++ b/tracks/app/controllers/todos_controller.rb @@ -2,13 +2,17 @@ class TodosController < ApplicationController helper :todos - append_before_filter :init, :except => [ :destroy, :completed, :completed_archive, :check_deferred ] - append_before_filter :get_todo_from_params, :only => [ :edit, :toggle_check, :show, :update, :destroy ] skip_before_filter :login_required, :only => [:index] prepend_before_filter :login_or_feed_token_required, :only => [:index] + append_before_filter :init, :except => [ :destroy, :completed, :completed_archive, :check_deferred ] + append_before_filter :get_todo_from_params, :only => [ :edit, :toggle_check, :show, :update, :destroy ] + + prepend_before_filter :enable_mobile_content_negotiation + after_filter :restore_content_type_for_mobile + session :off, :only => :index, :if => Proc.new { |req| is_feed_request(req) } - layout 'standard' + layout proc{ |controller| controller.mobile? ? "mobile" : "standard" } def index @projects = @user.projects.find(:all, :include => [ :todos ]) @@ -17,20 +21,29 @@ class TodosController < ApplicationController @contexts_to_show = @contexts.reject {|x| x.hide? } respond_to do |format| - format.html &render_todos_html - format.xml { render :action => 'list.rxml', :layout => false } + format.html &render_todos_html + format.m &render_todos_mobile + format.xml { render :action => 'list.rxml', :layout => false } format.rss &render_rss_feed format.atom &render_atom_feed format.text &render_text_feed format.ics &render_ical_feed end end - + + def new + @projects = @user.projects.find(:all) + @contexts = @user.contexts.find(:all) + respond_to do |format| + format.m { render :action => "new_mobile" } + end + end + def create @todo = @user.todos.build p = params['request'] || params - if p['todo']['show_from'] + if p['todo']['show_from'] && !mobile? p['todo']['show_from'] = parse_date_per_user_prefs(p['todo']['show_from']) end @@ -60,34 +73,46 @@ class TodosController < ApplicationController end if @todo.due? - @todo.due = parse_date_per_user_prefs(p['todo']['due']) + @todo.due = parse_date_per_user_prefs(p['todo']['due']) unless mobile? else @todo.due = "" end @saved = @todo.save if @saved - @todo.tag_with(params[:tag_list],@user) + @todo.tag_with(params[:tag_list],@user) if params[:tag_list] @todo.reload end - respond_to do |wants| - wants.html { redirect_to :action => "index" } - wants.js do + respond_to do |format| + format.html { redirect_to :action => "index" } + format.m do if @saved - determine_down_count + redirect_to :action => "index", :format => :m + else + render :action => "new", :format => :m end + end + format.js do + determine_down_count if @saved render :action => 'create' end - wants.xml { render :xml => @todo.to_xml( :root => 'todo', :except => :user_id ) } + format.xml { render :xml => @todo.to_xml( :root => 'todo', :except => :user_id ) } end end def edit + @projects = @user.projects.find(:all) + @contexts = @user.contexts.find(:all) end def show respond_to do |format| + format.m do + @projects = @user.projects.find(:all) + @contexts = @user.contexts.find(:all) + render :action => 'show_mobile' + end format.xml { render :xml => @todo.to_xml( :root => 'todo', :except => :user_id ) } end end @@ -120,7 +145,7 @@ class TodosController < ApplicationController end def update - @todo.tag_with(params[:tag_list],@user) + @todo.tag_with(params[:tag_list],@user) if params[:tag_list] @original_item_context_id = @todo.context_id @original_item_project_id = @todo.project_id @original_item_was_deferred = @todo.deferred? @@ -160,6 +185,10 @@ class TodosController < ApplicationController params['todo']['show_from'] = parse_date_per_user_prefs(params['todo']['show_from']) end + if params['done'] == '1' && !@todo.completed? + @todo.complete! + end + @saved = @todo.update_attributes params["todo"] @context_changed = @original_item_context_id != @todo.context_id @todo_was_activated_from_deferred_state = @original_item_was_deferred && @todo.active? @@ -167,6 +196,16 @@ class TodosController < ApplicationController @project_changed = @original_item_project_id != @todo.project_id if (@project_changed && !@original_item_project_id.nil?) then @remaining_undone_in_project = @user.projects.find(@original_item_project_id).not_done_todo_count; end determine_down_count + respond_to do |format| + format.js + format.m do + if @saved + redirect_to formatted_todos_path(:m) + else + render :action => "edit", :format => :m + end + end + end end def destroy @@ -175,9 +214,9 @@ class TodosController < ApplicationController @project_id = @todo.project_id @saved = @todo.destroy - respond_to do |wants| + respond_to do |format| - wants.html do + format.html do if @saved notify :notice, "Successfully deleted next action", 2.0 redirect_to :action => 'index' @@ -185,9 +224,9 @@ class TodosController < ApplicationController notify :error, "Failed to delete the action", 2.0 redirect_to :action => 'index' end - end + end - wants.js do + format.js do if @saved determine_down_count source_view do |from| @@ -199,7 +238,7 @@ class TodosController < ApplicationController render end - wants.xml { render :text => '200 OK. Action deleted.', :status => 200 } + format.xml { render :text => '200 OK. Action deleted.', :status => 200 } end end @@ -236,6 +275,16 @@ class TodosController < ApplicationController end end + def filter_to_context + context = @user.contexts.find(params['context']['id']) + redirect_to formatted_context_todos_path(context, :m) + end + + def filter_to_project + project = @user.projects.find(params['project']['id']) + redirect_to formatted_project_todos_path(project, :m) + end + # /todos/tag/[tag_name] shows all the actions tagged with tag_name # def tag @@ -266,15 +315,47 @@ class TodosController < ApplicationController end - private + # Here's the concept behind this "mobile content negotiation" hack: + # In addition to the main, AJAXy Web UI, Tracks has a lightweight + # low-feature 'mobile' version designed to be suitablef or use + # from a phone or PDA. It makes some sense that tne pages of that + # mobile version are simply alternate representations of the same + # Todo resources. The implementation goal was to treat mobile + # as another format and be able to use respond_to to render both + # versions. Unfortunately, I ran into a lot of trouble simply + # registering a new mime type 'text/html' with format :m because + # :html already is linked to that mime type and the new + # registration was forcing all html requests to be rendered in + # the mobile view. The before_filter and after_filter hackery + # below accomplishs that implementation goal by using a 'fake' + # mime type during the processing and then setting it to + # 'text/html' in an 'after_filter' -LKM 2007-04-01 + def mobile? + return params[:format] == 'm' || response.content_type == MOBILE_CONTENT_TYPE + end + def enable_mobile_content_negotiation + if mobile? + request.accepts.unshift(Mime::Type::lookup(MOBILE_CONTENT_TYPE)) + end + end + + def restore_content_type_for_mobile + if mobile? + response.content_type = 'text/html' + end + end + + private + + def get_todo_from_params @todo = @user.todos.find(params['id']) end def init @source_view = params['_source_view'] || 'todo' - init_data_for_sidebar + init_data_for_sidebar unless mobile? init_todos end @@ -321,13 +402,13 @@ class TodosController < ApplicationController def with_parent_resource_scope(&block) if (params[:context_id]) - context = @user.contexts.find_by_params(params) - Todo.with_scope :find => {:conditions => ['todos.context_id = ?', context.id]} do + @context = @user.contexts.find_by_params(params) + Todo.with_scope :find => {:conditions => ['todos.context_id = ?', @context.id]} do yield end elsif (params[:project_id]) - project = @user.projects.find_by_params(params) - Todo.with_scope :find => {:conditions => ['todos.project_id = ?', project.id]} do + @project = @user.projects.find_by_params(params) + Todo.with_scope :find => {:conditions => ['todos.project_id = ?', @project.id]} do yield end else @@ -350,12 +431,26 @@ class TodosController < ApplicationController with_feed_query_scope do with_parent_resource_scope do with_limit_scope do + + if mobile? + + @todos, @page = @user.todos.paginate(:all, + :conditions => ['state = ?', 'active' ], :include => [:context], + :order => 'due IS NULL, due ASC, todos.created_at ASC', + :page => params[:page], :per_page => 6) + @pagination_params = { :format => :m } + @pagination_params[:context_id] = @context.to_param if @context + @pagination_params[:project_id] = @project.to_param if @project + + else + + # Exclude hidden projects from count on home page + @todos = @user.todos.find(:all, :conditions => ['todos.state = ? or todos.state = ?', 'active', 'complete'], :include => [ :project, :context, :tags ]) - # Exclude hidden projects from count on home page - @todos = @user.todos.find(:all, :conditions => ['todos.state = ? or todos.state = ?', 'active', 'complete'], :include => [ :project, :context, :tags ]) - - # Exclude hidden projects from the home page - @not_done_todos = @user.todos.find(:all, :conditions => ['todos.state = ?', 'active'], :order => "todos.due IS NULL, todos.due ASC, todos.created_at ASC", :include => [ :project, :context, :tags ]) + # Exclude hidden projects from the home page + @not_done_todos = @user.todos.find(:all, :conditions => ['todos.state = ?', 'active'], :order => "todos.due IS NULL, todos.due ASC, todos.created_at ASC", :include => [ :project, :context, :tags ]) + + end end end @@ -415,6 +510,23 @@ class TodosController < ApplicationController render end end + + def render_todos_mobile + lambda do + @page_title = "All actions" + if @context + @page_title += " in context #{@context.name}" + @down_count = @context.not_done_todo_count + elsif @project + @page_title += " in project #{@project.name}" + @down_count = @project.not_done_todo_count + else + determine_down_count + end + + render :action => 'index_mobile' + end + end def render_rss_feed lambda do diff --git a/tracks/app/helpers/todos_helper.rb b/tracks/app/helpers/todos_helper.rb index 7c26ed62..77d79673 100644 --- a/tracks/app/helpers/todos_helper.rb +++ b/tracks/app/helpers/todos_helper.rb @@ -185,6 +185,11 @@ module TodosHelper split_notes = notes.split(/\n/) joined_notes = split_notes.join("\\n") end + + def formatted_pagination(total, per_page) + s = will_paginate(@down_count, 6) + (s.gsub /(<\/[^<]+>)/, '\1 ').chomp(' ') + end private diff --git a/tracks/app/views/mobile/_mobile_actions.rhtml b/tracks/app/views/mobile/_mobile_actions.rhtml deleted file mode 100644 index a19d5278..00000000 --- a/tracks/app/views/mobile/_mobile_actions.rhtml +++ /dev/null @@ -1,35 +0,0 @@ -<%= render_flash %> -<% if @todos.length == 0 -%> -

There are no incomplete actions in this <%= @type %>

-<% else -%> -