From 1bada8f5a3656869e4e74202d3edfb915401da9b Mon Sep 17 00:00:00 2001
From: lukemelia <lukemelia@a4c988fc-2ded-0310-b66e-134b36920a42>
Date: Wed, 28 Feb 2007 05:36:10 +0000
Subject: [PATCH] Introduced separation of projects by state (Active, Hidden,
 Completed) on the main Projects page. There are still some Ajax corner cases
 to work out, but it's perfectly usable. Closes #460.

I also refactored change password code.



git-svn-id: http://www.rousette.org.uk/svn/tracks-repos/trunk@464 a4c988fc-2ded-0310-b66e-134b36920a42
---
 tracks/app/controllers/projects_controller.rb | 33 ++++++++---------
 tracks/app/controllers/users_controller.rb    | 25 ++++---------
 tracks/app/helpers/projects_helper.rb         | 16 ++++-----
 tracks/app/models/project.rb                  |  2 +-
 tracks/app/models/user.rb                     |  8 +++++
 .../app/views/contexts/_context_listing.rhtml |  2 +-
 tracks/app/views/projects/_project_form.rhtml |  1 -
 .../app/views/projects/_project_listing.rhtml |  4 +--
 .../views/projects/_project_state_group.rhtml |  7 ++++
 tracks/app/views/projects/create.rjs          |  4 +--
 tracks/app/views/projects/index.rhtml         | 13 +++----
 tracks/app/views/projects/update.rjs          | 14 ++++++--
 tracks/db/schema.rb                           | 36 +++++++++----------
 tracks/public/stylesheets/standard.css        | 13 +++----
 .../functional/projects_controller_test.rb    |  4 +--
 .../test/functional/users_controller_test.rb  |  6 ++--
 tracks/test/unit/user_test.rb                 |  8 +++++
 17 files changed, 103 insertions(+), 93 deletions(-)
 create mode 100644 tracks/app/views/projects/_project_state_group.rhtml

diff --git a/tracks/app/controllers/projects_controller.rb b/tracks/app/controllers/projects_controller.rb
index b73ddb57..9aa7de1b 100644
--- a/tracks/app/controllers/projects_controller.rb
+++ b/tracks/app/controllers/projects_controller.rb
@@ -65,6 +65,8 @@ class ProjectsController < ApplicationController
   def update
     params['project'] ||= {}
     if params['project']['state']
+      @state_changed = @project.state != params['project']['state']
+      logger.info "@state_changed: #{@project.state} == #{params['project']['state']} != #{@state_changed}"
       @project.transition_to(params['project']['state'])
       params['project'].delete('state')
     end
@@ -81,6 +83,9 @@ class ProjectsController < ApplicationController
         else
           @project_not_done_counts[@project.id] = @project.reload().not_done_todo_count(:include_project_hidden_todos => true)
         end
+        @active_projects_count = @user.projects.count(:conditions => "state = 'active'")
+        @hidden_projects_count = @user.projects.count(:conditions => "state = 'hidden'")
+        @completed_projects_count = @user.projects.count(:conditions => "state = 'completed'")
         render
       elsif boolean_param('update_status')
         render :action => 'update_status'
@@ -101,15 +106,13 @@ class ProjectsController < ApplicationController
     end
   end
   
-  # Methods for changing the sort order of the projects in the list
-  #
   def order
-    params["list-projects"].each_with_index do |id, position|
-      if check_user_matches_project_user(id)
-        Project.update(id, :position => position + 1)
-      end
-    end
+    project_ids = params["list-active-projects"] || params["list-hidden-projects"] || params["list-completed-projects"]    
+    projects = @user.projects.update_positions( project_ids )
     render :nothing => true
+  rescue
+    notify :error, $!
+    redirect_to :action => 'index'
   end
   
   protected
@@ -118,6 +121,9 @@ class ProjectsController < ApplicationController
       lambda do
         init_project_hidden_todo_counts
         @page_title = "TRACKS::List Projects"
+        @active_projects = @projects.select{ |p| p.active? }
+        @hidden_projects = @projects.select{ |p| p.hidden? }
+        @completed_projects = @projects.select{ |p| p.completed? }
         render
       end
     end
@@ -148,18 +154,7 @@ class ProjectsController < ApplicationController
       @project = @user.projects.find_by_params(params)
       render :text => 'Project not found', :status => 404 if @project.nil?
     end
-    
-    def check_user_matches_project_user(id)
-      @project = Project.find_by_id_and_user_id(id, @user.id)
-      if @user == @project.user
-        return @project
-      else
-        @project = nil
-        notify :warning, "Project and session user mis-match: #{@project.user_id} and #{@user.id}!"
-        render :text => ''
-      end
-    end
-    
+        
     def check_user_return_item
       item = Todo.find( params['id'] )
       if @user == item.user
diff --git a/tracks/app/controllers/users_controller.rb b/tracks/app/controllers/users_controller.rb
index b6f0e625..da371e1d 100644
--- a/tracks/app/controllers/users_controller.rb
+++ b/tracks/app/controllers/users_controller.rb
@@ -138,12 +138,12 @@ class UsersController < ApplicationController
   end
   
   def update_password
-    if do_change_password_for(@user)
-      redirect_to :controller => 'preferences'
-    else
-      redirect_to :action => 'change_password'
-      notify :warning, "There was a problem saving the password. Please retry."
-    end
+    @user.change_password(params[:updateuser][:password], params[:updateuser][:password_confirmation])
+    notify :notice, "Password updated."
+    redirect_to :controller => 'preferences'
+  rescue Exception => error
+    notify :error, error.message
+    redirect_to :action => 'change_password'
   end
 
   def change_auth_type
@@ -224,19 +224,6 @@ class UsersController < ApplicationController
     redirect_to :controller => 'preferences', :action => 'index'
   end
   
-  protected
-  
-  def do_change_password_for(user)
-    user.change_password(params[:updateuser][:password], params[:updateuser][:password_confirmation])
-    if user.save
-      notify :notice, "Password updated."
-      return true
-    else
-      notify :error, 'There was a problem saving the password. Please retry.'
-      return false
-    end
-  end
-
   private
     
   def get_new_user
diff --git a/tracks/app/helpers/projects_helper.rb b/tracks/app/helpers/projects_helper.rb
index f91d8a61..e0d7f482 100644
--- a/tracks/app/helpers/projects_helper.rb
+++ b/tracks/app/helpers/projects_helper.rb
@@ -1,12 +1,12 @@
 module ProjectsHelper
 
-def get_listing_sortable_options
-  {
-    :tag => 'div',
-    :handle => 'handle',
-    :complete => visual_effect(:highlight, 'list-projects'),
-    :url => order_projects_path
-  }
-end
+  def get_listing_sortable_options(list_container_id)
+    {
+      :tag => 'div',
+      :handle => 'handle',
+      :complete => visual_effect(:highlight, list_container_id),
+      :url => order_projects_path
+    }
+  end
 
 end
diff --git a/tracks/app/models/project.rb b/tracks/app/models/project.rb
index 5ea67b50..84022914 100644
--- a/tracks/app/models/project.rb
+++ b/tracks/app/models/project.rb
@@ -9,7 +9,7 @@ class Project < ActiveRecord::Base
   validates_does_not_contain :name, :string => '/', :message => "cannot contain the slash ('/') character"
   validates_does_not_contain :name, :string => ',', :message => "cannot contain the comma (',') character"
 
-  acts_as_list :scope => :user
+  acts_as_list :scope => 'user_id = #{user_id} AND state = \'#{state}\''
   acts_as_state_machine :initial => :active, :column => 'state'
   extend NamePartFinder
   include Tracks::TodoList
diff --git a/tracks/app/models/user.rb b/tracks/app/models/user.rb
index 2ceccb2c..d884308f 100644
--- a/tracks/app/models/user.rb
+++ b/tracks/app/models/user.rb
@@ -30,6 +30,13 @@ class User < ActiveRecord::Base
                   find_by_url_friendly_name(params['project'])
                 end
               end
+              def update_positions(project_ids)
+                project_ids.each_with_index do |id, position|
+                  project = self.detect { |p| p.id == id.to_i }
+                  raise "Project id #{id} not associated with user id #{@user.id}." if project.nil?
+                  project.update_attribute(:position, position + 1)
+                end
+              end
             end
   has_many :todos,
            :order => 'completed_at DESC, todos.created_at DESC',
@@ -110,6 +117,7 @@ class User < ActiveRecord::Base
   def change_password(pass,pass_confirm)
     self.password = pass
     self.password_confirmation = pass_confirm
+    save!
   end
 
   def crypt_word
diff --git a/tracks/app/views/contexts/_context_listing.rhtml b/tracks/app/views/contexts/_context_listing.rhtml
index c824d14e..8af019ff 100644
--- a/tracks/app/views/contexts/_context_listing.rhtml
+++ b/tracks/app/views/contexts/_context_listing.rhtml
@@ -1,6 +1,6 @@
 <%  context = context_listing %>
 <div id="<%= dom_id(context, "container") %>" class="list">
-  <div id="<%= dom_id(context) %>" class="context even_row" style="display:'';">
+  <div id="<%= dom_id(context) %>" class="context sortable_row" style="display:'';">
     <div class="position">
       <span class="handle">DRAG</span>
     </div>
diff --git a/tracks/app/views/projects/_project_form.rhtml b/tracks/app/views/projects/_project_form.rhtml
index 117b3435..f3917ee3 100644
--- a/tracks/app/views/projects/_project_form.rhtml
+++ b/tracks/app/views/projects/_project_form.rhtml
@@ -17,4 +17,3 @@
    <% end %>
   </td>
 </tr>
-<% @project = nil %>
\ No newline at end of file
diff --git a/tracks/app/views/projects/_project_listing.rhtml b/tracks/app/views/projects/_project_listing.rhtml
index fa9706ce..7a68955e 100644
--- a/tracks/app/views/projects/_project_listing.rhtml
+++ b/tracks/app/views/projects/_project_listing.rhtml
@@ -1,6 +1,6 @@
 <%  project = project_listing %>
 <div id="<%= dom_id(project, "container") %>" class="list">
-  <div id="<%= dom_id(project) %>" class="project even_row" style="display:'';">
+  <div id="<%= dom_id(project) %>" class="project sortable_row" style="display:'';">
     <div class="position">
       <span class="handle">DRAG</span>
     </div>  
@@ -16,7 +16,7 @@
            page << "                               evalScripts:true, method:'delete'}); };"
          end -%>
          <a class="edit_project_button" href="#"><%= image_tag( "blank.png", :title => "Edit project", :class=>"edit_item") %></a>
-         <%= apply_behavior 'a.edit_project_button:click', :prevent_default => true do |page, element|
+         <%= apply_behavior 'a.edit_project_button:click', {:prevent_default => true, :external => true} do |page, element|
               element.up('.project').toggle 
               editform = element.up('.list').down('.edit-form')
               editform.toggle 
diff --git a/tracks/app/views/projects/_project_state_group.rhtml b/tracks/app/views/projects/_project_state_group.rhtml
new file mode 100644
index 00000000..e6d6b47f
--- /dev/null
+++ b/tracks/app/views/projects/_project_state_group.rhtml
@@ -0,0 +1,7 @@
+<div class="project-state-group">
+  <h2 <%= " style=\"display:none\"" if project_state_group.empty? %>><span id="<%= state %>-projects-count" class="badge"><%= project_state_group.length %></span><%= state.titlecase %> Projects</h2>
+  <div id="list-<%= state %>-projects">
+    <%= render :partial => 'project_listing', :collection => project_state_group %>
+  </div>
+  <%= sortable_element "list-#{state}-projects", get_listing_sortable_options("list-#{state}-projects") %>
+</div>
\ No newline at end of file
diff --git a/tracks/app/views/projects/create.rjs b/tracks/app/views/projects/create.rjs
index 553695dc..eabeb9fd 100644
--- a/tracks/app/views/projects/create.rjs
+++ b/tracks/app/views/projects/create.rjs
@@ -1,6 +1,6 @@
 if @saved
-  page.insert_html :bottom, "list-projects", :partial => 'project_listing', :locals => { :project_listing => @project }
-  page.sortable "list-projects", get_listing_sortable_options
+  page.insert_html :bottom, "list-active-projects", :partial => 'project_listing', :locals => { :project_listing => @project }
+  page.sortable "list-active-projects", get_listing_sortable_options('list-active-projects')
   page.call "Form.reset", "project-form"
   page.call "Form.focusFirstElement", "project-form"
 else
diff --git a/tracks/app/views/projects/index.rhtml b/tracks/app/views/projects/index.rhtml
index a32d5197..71e15689 100644
--- a/tracks/app/views/projects/index.rhtml
+++ b/tracks/app/views/projects/index.rhtml
@@ -1,17 +1,18 @@
 <div id="display_box">
-  <div id="list-projects">
-    <%= render :partial => 'project_listing', :collection => @projects %>
-  </div>
-  <%= sortable_element 'list-projects', get_listing_sortable_options %>
+  <%= render :partial => 'project_state_group', :object => @active_projects, :locals => { :state => 'active'} %>
+  <%= render :partial => 'project_state_group', :object => @hidden_projects, :locals => { :state => 'hidden'} %>
+  <%= render :partial => 'project_state_group', :object => @completed_projects, :locals => { :state => 'completed'} %>
 </div>
 
 <div id="input_box">
-  <a href="#" onClick="Element.toggle('project_new'); Form.focusFirstElement('project-form');return false" accesskey="n" title="Create a new project">Create new project &#187;</a>
+  <%= link_to_function("Create a new project &#187;",
+                       "Element.toggle('project_new');Form.focusFirstElement('project-form');",
+                       {:title => "Add a next action", :accesskey => "n"}) %>
 
   <div id="project_new" class="project_new" style="display:none">
     <% form_remote_tag :url => projects_path, :method => :post,
                        :html=> { :id=>'project-form', :name=>'project', :class => 'inline-form' } do -%>
-  
+
       <div id="status"><%= error_messages_for('project') %></div>
 
       <label for="project_name">Name:</label><br />
diff --git a/tracks/app/views/projects/update.rjs b/tracks/app/views/projects/update.rjs
index f7285864..b4cd17b9 100644
--- a/tracks/app/views/projects/update.rjs
+++ b/tracks/app/views/projects/update.rjs
@@ -1,2 +1,12 @@
-page.replace_html dom_id(@project, 'container'), :partial => 'project_listing', :object => @project
-page.sortable "list-projects", get_listing_sortable_options
\ No newline at end of file
+status_message = 'Project saved'
+page.notify :notice, status_message, 5.0
+if @state_changed
+  page[dom_id(@project, 'container')].remove
+  page.insert_html :bottom, "list-#{@project.state}-projects", :partial => 'project_listing', :object => @project
+else
+  page.replace_html dom_id(@project, 'container'), :partial => 'project_listing', :object => @project
+end
+page.sortable "list-#{@project.state}-projects", get_listing_sortable_options("list-#{@project.state}-projects")
+page.replace_html "active-projects-count", @active_projects_count
+page.replace_html "hidden-projects-count", @hidden_projects_count
+page.replace_html "completed-projects-count", @completed_projects_count
diff --git a/tracks/db/schema.rb b/tracks/db/schema.rb
index 331a36f8..3bcd23ef 100644
--- a/tracks/db/schema.rb
+++ b/tracks/db/schema.rb
@@ -5,10 +5,10 @@
 ActiveRecord::Schema.define(:version => 30) do
 
   create_table "contexts", :force => true do |t|
-    t.column "name",       :string,   :default => "",    :null => false
-    t.column "position",   :integer,                     :null => false
-    t.column "hide",       :boolean,  :default => false
-    t.column "user_id",    :integer,  :default => 1
+    t.column "name",       :string,                :default => "", :null => false
+    t.column "hide",       :integer,  :limit => 4, :default => 0,  :null => false
+    t.column "position",   :integer,               :default => 0,  :null => false
+    t.column "user_id",    :integer,               :default => 0,  :null => false
     t.column "created_at", :datetime
     t.column "updated_at", :datetime
   end
@@ -17,8 +17,8 @@ ActiveRecord::Schema.define(:version => 30) do
   add_index "contexts", ["user_id", "name"], :name => "index_contexts_on_user_id_and_name"
 
   create_table "notes", :force => true do |t|
-    t.column "user_id",    :integer,  :null => false
-    t.column "project_id", :integer,  :null => false
+    t.column "user_id",    :integer,  :default => 0, :null => false
+    t.column "project_id", :integer,  :default => 0, :null => false
     t.column "body",       :text
     t.column "created_at", :datetime
     t.column "updated_at", :datetime
@@ -44,7 +44,7 @@ ActiveRecord::Schema.define(:version => 30) do
   end
 
   create_table "preferences", :force => true do |t|
-    t.column "user_id",                            :integer,                                                           :null => false
+    t.column "user_id",                            :integer,               :default => 0,                              :null => false
     t.column "date_format",                        :string,  :limit => 40, :default => "%d/%m/%Y",                     :null => false
     t.column "week_starts",                        :integer,               :default => 0,                              :null => false
     t.column "show_number_completed",              :integer,               :default => 5,                              :null => false
@@ -65,8 +65,8 @@ ActiveRecord::Schema.define(:version => 30) do
 
   create_table "projects", :force => true do |t|
     t.column "name",        :string,                 :default => "",       :null => false
-    t.column "position",    :integer,                                      :null => false
-    t.column "user_id",     :integer,                :default => 1
+    t.column "position",    :integer,                :default => 0,        :null => false
+    t.column "user_id",     :integer,                :default => 0,        :null => false
     t.column "description", :text
     t.column "state",       :string,   :limit => 20, :default => "active", :null => false
     t.column "created_at",  :datetime
@@ -82,7 +82,7 @@ ActiveRecord::Schema.define(:version => 30) do
     t.column "updated_at", :datetime
   end
 
-  add_index "sessions", ["session_id"], :name => "index_sessions_on_session_id"
+  add_index "sessions", ["session_id"], :name => "sessions_session_id_index"
 
   create_table "taggings", :force => true do |t|
     t.column "taggable_id",   :integer
@@ -102,16 +102,16 @@ ActiveRecord::Schema.define(:version => 30) do
   add_index "tags", ["name"], :name => "index_tags_on_name"
 
   create_table "todos", :force => true do |t|
-    t.column "context_id",   :integer,                                         :null => false
-    t.column "project_id",   :integer
-    t.column "description",  :string,                 :default => "",          :null => false
+    t.column "context_id",   :integer,                 :default => 0,           :null => false
+    t.column "description",  :string,   :limit => 100, :default => "",          :null => false
     t.column "notes",        :text
     t.column "created_at",   :datetime
     t.column "due",          :date
     t.column "completed_at", :datetime
-    t.column "user_id",      :integer,                :default => 1
+    t.column "project_id",   :integer
+    t.column "user_id",      :integer,                 :default => 0,           :null => false
     t.column "show_from",    :date
-    t.column "state",        :string,   :limit => 20, :default => "immediate", :null => false
+    t.column "state",        :string,   :limit => 20,  :default => "immediate", :null => false
   end
 
   add_index "todos", ["user_id", "state"], :name => "index_todos_on_user_id_and_state"
@@ -121,10 +121,10 @@ ActiveRecord::Schema.define(:version => 30) do
   add_index "todos", ["user_id", "context_id"], :name => "index_todos_on_user_id_and_context_id"
 
   create_table "users", :force => true do |t|
-    t.column "login",       :string,  :limit => 80, :default => "",         :null => false
-    t.column "password",    :string,  :limit => 40, :default => "",         :null => false
+    t.column "login",       :string,  :limit => 80
+    t.column "password",    :string,  :limit => 40
     t.column "word",        :string
-    t.column "is_admin",    :boolean,               :default => false,      :null => false
+    t.column "is_admin",    :integer, :limit => 4,  :default => 0,          :null => false
     t.column "first_name",  :string
     t.column "last_name",   :string
     t.column "auth_type",   :string,                :default => "database", :null => false
diff --git a/tracks/public/stylesheets/standard.css b/tracks/public/stylesheets/standard.css
index 88ab82b7..8567ca3e 100644
--- a/tracks/public/stylesheets/standard.css
+++ b/tracks/public/stylesheets/standard.css
@@ -191,7 +191,7 @@ div#input_box {
       margin: 0% 5% 0% 60%;
       padding: 0px 15px 0px 0px;
       }
-      
+
 #input_box h2 {
       color: #999;
       
@@ -472,19 +472,14 @@ li {
 #sidebar li {
       padding: auto;
       }
-.even_row {
+.sortable_row {
       background: #fff;
       _background: transparent;
-      padding: 5px 5px 5px 10px;
+      padding: 4px 4px 4px 8px;
       margin: 2px 2px;
       border: 1px solid #ccc;
       }
 
-.odd_row {
-      background: #EDF3FE;
-      padding: 5px 5px 5px 10px;
-      }
-
 .edit-form {
       background: #ccc;
       padding: 5px;
@@ -526,7 +521,7 @@ div.buttons, div.buttons a, div.buttons a:hover {
     background-color: transparent;
     }
 
-div#list-projects, div#list-contexts {
+div#list-active-projects, div#list-hidden-projects, div#list-completed-projects, div#list-contexts {
     border: 1px solid #999;
     }
     
diff --git a/tracks/test/functional/projects_controller_test.rb b/tracks/test/functional/projects_controller_test.rb
index 7bb799ca..bfe17a19 100644
--- a/tracks/test/functional/projects_controller_test.rb
+++ b/tracks/test/functional/projects_controller_test.rb
@@ -38,8 +38,8 @@ class ProjectsControllerTest < TodoContainerControllerTestBase
 
   def test_create_project_with_ajax_success_rjs
     ajax_create 'My New Project'
-    assert_rjs :insert_html, :bottom, "list-projects"
-    assert_rjs :sortable, 'list-projects', { :tag => 'div', :handle => 'handle', :complete => visual_effect(:highlight, 'list-projects'), :url => order_projects_path }
+    assert_rjs :insert_html, :bottom, "list-active-projects"
+    assert_rjs :sortable, 'list-active-projects', { :tag => 'div', :handle => 'handle', :complete => visual_effect(:highlight, 'list-active-projects'), :url => order_projects_path }
     # not yet sure how to write the following properly...
     assert_rjs :call, "Form.reset", "project-form"
     assert_rjs :call, "Form.focusFirstElement", "project-form"
diff --git a/tracks/test/functional/users_controller_test.rb b/tracks/test/functional/users_controller_test.rb
index f00a6e26..f6a95a3c 100644
--- a/tracks/test/functional/users_controller_test.rb
+++ b/tracks/test/functional/users_controller_test.rb
@@ -58,7 +58,7 @@ class UsersControllerTest < Test::Unit::TestCase
     assert_redirected_to :controller => 'preferences'
     @updated_user = User.find(users(:admin_user).id)
     assert_equal @updated_user.password, Digest::SHA1.hexdigest("#{Tracks::Config.salt}--newpassword--")
-    assert_equal flash[:notice], "Password updated."
+    assert_equal "Password updated.", flash[:notice]
   end
   
   def test_update_password_no_confirmation
@@ -68,7 +68,7 @@ class UsersControllerTest < Test::Unit::TestCase
     post :update_password, :updateuser => {:password => 'newpassword', :password_confirmation => 'wrong'}
     assert_redirected_to :controller => 'users', :action => 'change_password'
     assert users(:admin_user).save, false
-    assert_equal flash[:warning], 'There was a problem saving the password. Please retry.'
+    assert_equal 'Validation failed: Password doesn\'t match confirmation', flash[:error]
   end
   
   def test_update_password_validation_errors
@@ -81,7 +81,7 @@ class UsersControllerTest < Test::Unit::TestCase
     # For some reason, no errors are being raised now.
     #assert_equal 1, users(:admin_user).errors.count
     #assert_equal users(:admin_user).errors.on(:password), "is too short (min is 5 characters)"
-    assert_equal flash[:warning], 'There was a problem saving the password. Please retry.'
+    assert_equal 'Validation failed: Password is too short (minimum is 5 characters)', flash[:error]
   end
   
   # ============================================
diff --git a/tracks/test/unit/user_test.rb b/tracks/test/unit/user_test.rb
index 807dac38..e8e95608 100644
--- a/tracks/test/unit/user_test.rb
+++ b/tracks/test/unit/user_test.rb
@@ -143,5 +143,13 @@ class UserTest < Test::Unit::TestCase
     assert_equal @admin_user.login, @admin_user.to_param
   end
   
+  def test_change_password
+    assert_not_nil User.authenticate(@admin_user.login, "abracadabra")
+    @admin_user.change_password("foobar", "foobar")
+    @admin_user.reload
+    assert_nil User.authenticate(@admin_user.login, "abracadabra")
+    assert_not_nil User.authenticate(@admin_user.login, "foobar")
+  end
+  
 
 end