From 3a07010338a126fea81960d37b56098a51d06e07 Mon Sep 17 00:00:00 2001 From: Reinier Balt Date: Wed, 28 Sep 2011 13:54:50 +0200 Subject: [PATCH] first cleanups of review feature --- app/controllers/projects_controller.rb | 33 ++++++------------- app/helpers/projects_helper.rb | 13 +++----- app/models/project.rb | 24 +++++--------- app/views/projects/_project_state_group.rhtml | 10 ++++-- app/views/projects/review.html.erb | 8 ++--- config/routes.rb | 16 ++++----- ...0915100000_add_last_reviewed_to_project.rb | 4 ++- ...10915100001_add_next_review_preferences.rb | 4 ++- 8 files changed, 46 insertions(+), 66 deletions(-) diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 3e991bf6..306dd47b 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -2,7 +2,7 @@ class ProjectsController < ApplicationController helper :application, :todos, :notes before_filter :set_source_view - before_filter :set_project_from_params, :only => [:update, :destroy, :show, :edit] + before_filter :set_project_from_params, :only => [:update, :destroy, :show, :edit, :set_reviewed] before_filter :default_context_filter, :only => [:create, :update] skip_before_filter :login_required, :only => [:index] prepend_before_filter :login_or_feed_token_required, :only => [:index] @@ -34,34 +34,23 @@ class ProjectsController < ApplicationController end def review - ## select project that need reviewing - @projects_to_review = current_user.projects.select {|p| p.needs_review?(current_user)} - - ## select project that are stalled - @stalled_projects = current_user.projects.select {|p| p.stalled?} - - ## select project that are stalled - @blocked_projects = current_user.projects.select {|p| p.blocked?} - - ## select projects that are current - @current_projects = current_user.projects.select {|p| not(p.needs_review?(current_user))} - + @page_title = t('projects.list_reviews') + @projects = current_user.projects.all @contexts = current_user.contexts.all + @projects_to_review = current_user.projects.select {|p| p.needs_review?(current_user)} + @stalled_projects = current_user.projects.select {|p| p.stalled?} + @blocked_projects = current_user.projects.select {|p| p.blocked?} + @current_projects = current_user.projects.select {|p| not(p.needs_review?(current_user))} + init_not_done_counts(['project']) init_project_hidden_todo_counts(['project']) - if params[:only_active_with_no_next_actions] - @projects = current_user.projects.active.select { |p| count_undone_todos(p) == 0 } - else - @projects = current_user.projects.all - end + current_user.projects.cache_note_counts @page_title = t('projects.list_reviews') @count = @projects_to_review.count + @blocked_projects.count + @stalled_projects.count + @current_projects.count @no_projects = current_user.projects.empty? - current_user.projects.cache_note_counts @new_project = current_user.projects.build - render end def done @@ -79,12 +68,10 @@ class ProjectsController < ApplicationController @range_high = @range_low + @projects.size - 1 init_not_done_counts(['project']) - render end def set_reviewed - @project = current_user.projects.find(params[:id]) - @project.last_reviewed = Time.now + @project.last_reviewed = Time.zone.now @project.save redirect_to :action => 'show' end diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb index aadb3303..50ea79e9 100644 --- a/app/helpers/projects_helper.rb +++ b/app/helpers/projects_helper.rb @@ -8,7 +8,7 @@ module ProjectsHelper :url => order_projects_path } end - + def set_element_visible(id,test) if (test) page.show id @@ -30,7 +30,7 @@ module ProjectsHelper end html end - + def project_next_prev_mobile html = '' unless @previous_project.nil? @@ -61,7 +61,7 @@ module ProjectsHelper def summary(project) project_description = '' project_description += sanitize(markdown( project.description )) unless project.description.blank? - project_description += content_tag(:p, + project_description += content_tag(:p, "#{count_undone_todos_phrase(p)}. " + t('projects.project_state', :state => project.state) ) project_description @@ -69,12 +69,7 @@ module ProjectsHelper def needsreview_class(item) raise "item must be a Project " unless item.kind_of? Project - - if item.needs_review?(current_user) - return "needsreview" - else - return "needsnoreview" - end + return item.needs_review?(current_user) ? "needsreview" : "needsnoreview" end end diff --git a/app/models/project.rb b/app/models/project.rb index fa96744d..8b507207 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -109,28 +109,20 @@ class Project < ActiveRecord::Base def needs_review?(current_user) return true if last_reviewed.nil? - return (active? && (last_reviewed < current_user.time - current_user.prefs.review_period.days)) + return (active? && (last_reviewed < current_user.time - current_user.prefs.review_period.days)) end def blocked? ## mutually exclusive for stalled and blocked - return false if stalled? - return false if completed? - is_blocked = true - todos.each do |t| - is_blocked = false if (!t.completed? && !t.deferred? && !t.pending?) - end - return is_blocked + # blocked is uncompleted project with deferred or pending todos, but no next actions + return false if self.stalled? || self.completed? + return !self.todos.deferred_or_blocked.empty? end - + def stalled? - return true if todos.count == 0 - return false if completed? - is_stalled = true - todos.each do |t| - is_stalled = false if (!t.completed?) - end - return is_stalled + # stalled is active/hidden project with no active todos + return false if self.completed? + return self.todos.not_completed.empty? end diff --git a/app/views/projects/_project_state_group.rhtml b/app/views/projects/_project_state_group.rhtml index f37cb9fc..f052d125 100644 --- a/app/views/projects/_project_state_group.rhtml +++ b/app/views/projects/_project_state_group.rhtml @@ -1,16 +1,19 @@ <%- total_count ||= -1 total_count_string = total_count!=-1 ? " / #{total_count}" : "" + suppress_sort_menu ||= false + suppress_drag_handle ||= false -%>
>

<%= project_state_group.length%><%= total_count_string%> - <%= t('common.last' ) unless (state == 'review' || state == 'stalled' || state == 'blocked' || state == 'current')%> - <%= t('states.'+state+'_plural' )%> + <%= t('common.last' ) unless ( ['review','stalled','blocked','current'].include?(state) )%> + <%= t('states.'+state+'_plural' )%> <%= t('common.projects') %><%= total_count==-1 ? "" : " ("+link_to("Show all", done_projects_path)+")"%>

+ <% unless suppress_sort_menu %>
<%= t('common.sort.sort') %> 
<%= link_to(t("common.sort.alphabetically"), alphabetize_projects_path(:state => state), @@ -20,8 +23,9 @@ :id => "#{state}_actionize_link", :class => "actionize_link", :title => t('common.sort.by_task_count_title'), :x_confirm_message => t('common.sort.by_task_count_title_confirm')) %>
+ <% end %>
- <%= render :partial => 'projects/project_listing', :collection => project_state_group %> + <%= render :partial => 'projects/project_listing', :collection => project_state_group, :locals => {:suppress_drag_handle => suppress_drag_handle} %>
diff --git a/app/views/projects/review.html.erb b/app/views/projects/review.html.erb index ac6dc1ef..56073c27 100644 --- a/app/views/projects/review.html.erb +++ b/app/views/projects/review.html.erb @@ -1,7 +1,7 @@

<%= t('projects.no_projects') %>

- <%= render :partial => 'project_state_group', :object => @projects_to_review, :locals => { :state => 'review'} %> - <%= render :partial => 'project_state_group', :object => @stalled_projects, :locals => { :state => 'stalled'} %> - <%= render :partial => 'project_state_group', :object => @blocked_projects, :locals => { :state => 'blocked'} %> - <%= render :partial => 'project_state_group', :object => @current_projects, :locals => { :state => 'current'} %> + <%= render :partial => 'project_state_group', :object => @projects_to_review, :locals => { :state => 'review', :suppress_sort_menu => true, :suppress_drag_handle => true} %> + <%= render :partial => 'project_state_group', :object => @stalled_projects, :locals => { :state => 'stalled', :suppress_sort_menu => true, :suppress_drag_handle => true} %> + <%= render :partial => 'project_state_group', :object => @blocked_projects, :locals => { :state => 'blocked', :suppress_sort_menu => true, :suppress_drag_handle => true} %> + <%= render :partial => 'project_state_group', :object => @current_projects, :locals => { :state => 'current', :suppress_sort_menu => true, :suppress_drag_handle => true} %> diff --git a/config/routes.rb b/config/routes.rb index c642eb35..43a833d1 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -12,15 +12,15 @@ ActionController::Routing::Routes.draw do |map| contexts.resources :todos, :name_prefix => "context_" end - map.resources :projects, :collection => {:order => :post, :alphabetize => :post, :actionize => :post, :done => :get}, - :member => {:done_todos => :get, :all_done_todos => :get, :set_reviewed => :get} do |projects| - projects.resources :todos, :name_prefix => "project_" + map.resources :projects, + :collection => {:order => :post, :alphabetize => :post, :actionize => :post, :done => :get}, + :member => {:done_todos => :get, :all_done_todos => :get, :set_reviewed => :post} do |projects| + projects.resources :todos, :name_prefix => "project_" end - map.with_options :controller => :projects do |projects| - # projects.home '', :action => "index" - projects.review 'review', :action => :review - end + map.with_options :controller => :projects do |projects| + projects.review 'review', :action => :review + end map.resources :notes @@ -29,7 +29,6 @@ ActionController::Routing::Routes.draw do |map| :collection => {:check_deferred => :post, :filter_to_context => :post, :filter_to_project => :post, :done => :get, :all_done => :get } - map.with_options :controller => :todos do |todos| todos.home '', :action => "index" todos.tickler 'tickler.:format', :action => "list_deferred" @@ -104,7 +103,6 @@ ActionController::Routing::Routes.draw do |map| map.search 'search', :controller => 'search', :action => 'index' map.data 'data', :controller => 'data', :action => 'index' - map.connect '/selenium_helper/login', :controller => 'selenium_helper', :action => 'login' if Rails.env == 'test' Translate::Routes.translation_ui(map) if Rails.env != "production" # Install the default route as the lowest priority. diff --git a/db/migrate/20110915100000_add_last_reviewed_to_project.rb b/db/migrate/20110915100000_add_last_reviewed_to_project.rb index e6d84814..ec81a5f2 100644 --- a/db/migrate/20110915100000_add_last_reviewed_to_project.rb +++ b/db/migrate/20110915100000_add_last_reviewed_to_project.rb @@ -1,8 +1,10 @@ class AddLastReviewedToProject < ActiveRecord::Migration + def self.up add_column :projects, :last_reviewed, :timestamp - execute 'update projects set last_reviewed = created_at where last_reviewed IS NULL' + execute 'UPDATE projects SET last_reviewed = created_at WHERE last_reviewed IS NULL' end + def self.down remove_column :projects, :last_reviewed end diff --git a/db/migrate/20110915100001_add_next_review_preferences.rb b/db/migrate/20110915100001_add_next_review_preferences.rb index 633c765d..3d48bdd1 100644 --- a/db/migrate/20110915100001_add_next_review_preferences.rb +++ b/db/migrate/20110915100001_add_next_review_preferences.rb @@ -1,8 +1,10 @@ class AddNextReviewPreferences < ActiveRecord::Migration + def self.up add_column :preferences, :review_period, :integer, :default => 14, :null => false end + def self.down - remove_column :preferences, :review_period + remove_column :preferences, :review_period end end