Merged r4645 to r4651 from trunk.

git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/branches/1.1-stable@4660 e93f8b46-1217-0410-a6f0-8f06a7374b81

app/controllers/projects_controller.rb

@@ -32,9 +32,6 @@ class ProjectsController < ApplicationController
     end
   end
 
-  # TODO: convert to PUT only
-  verify :method => [:post, :put], :only => :update, :render => {:nothing => true, :status => :method_not_allowed }
-
   helper :sort
   include SortHelper
   helper :custom_fields
@@ -71,13 +68,13 @@ class ProjectsController < ApplicationController
     @project = Project.new(params[:project])
   end
 
+  verify :method => :post, :only => :create, :render => {:nothing => true, :status => :method_not_allowed }
   def create
     @issue_custom_fields = IssueCustomField.find(:all, :order => "#{CustomField.table_name}.position")
     @trackers = Tracker.all
     @project = Project.new
     @project.safe_attributes = params[:project]
 
-    @project.enabled_module_names = params[:enabled_modules] if params[:enabled_modules]
     if validate_parent_id && @project.save
       @project.set_allowed_parent!(params[:project]['parent_id']) if params[:project].has_key?('parent_id')
       # Add current user as a project member if he is not admin
@@ -184,6 +181,8 @@ class ProjectsController < ApplicationController
   def edit
   end
 
+  # TODO: convert to PUT only
+  verify :method => [:post, :put], :only => :update, :render => {:nothing => true, :status => :method_not_allowed }
   def update
     @project.safe_attributes = params[:project]
     if validate_parent_id && @project.save
@@ -206,8 +205,9 @@ class ProjectsController < ApplicationController
     end
   end
 
+  verify :method => :post, :only => :modules, :render => {:nothing => true, :status => :method_not_allowed }
   def modules
-    @project.enabled_module_names = params[:enabled_modules]
+    @project.enabled_module_names = params[:enabled_module_names]
     flash[:notice] = l(:notice_successful_update)
     redirect_to :action => 'settings', :id => @project, :tab => 'modules'
   end

app/models/project.rb

@@ -66,7 +66,7 @@ class Project < ActiveRecord::Base
                 :url => Proc.new {|o| {:controller => 'projects', :action => 'show', :id => o}},
                 :author => nil
 
-  attr_protected :status, :enabled_module_names
+  attr_protected :status
   
   validates_presence_of :name, :identifier
   validates_uniqueness_of :identifier
@@ -533,6 +533,9 @@ class Project < ActiveRecord::Base
     'tracker_ids',
     'issue_custom_field_ids'
 
+  safe_attributes 'enabled_module_names',
+    :if => lambda {|project, user| project.new_record? || user.allowed_to?(:select_project_modules, project) }
+  
   # Returns an array of projects that are in this project's hierarchy
   #
   # Example: parents, children, siblings

app/views/projects/_form.rhtml

@@ -23,8 +23,22 @@
 <%= call_hook(:view_projects_form, :project => @project, :form => f) %>
 </div>
 
+<% if @project.new_record? %>
+<fieldset class="box"><legend><%= l(:label_module_plural) %></legend>
+<% Redmine::AccessControl.available_project_modules.each do |m| %>
+    <label class="floating">
+    <%= check_box_tag 'project[enabled_module_names][]', m, @project.module_enabled?(m), :id => "project_enabled_module_names_#{m}" %>
+    <%= l_or_humanize(m, :prefix => "project_module_") %>
+    </label>
+<% end %>
+<%= hidden_field_tag 'project[enabled_module_names][]', '' %>
+<%= javascript_tag 'observeProjectModules()' %>
+</fieldset>
+<% end %>
+
+<% if @project.new_record? || @project.module_enabled?('issue_tracking') %>
 <% unless @trackers.empty? %>
-<fieldset class="box"><legend><%=l(:label_tracker_plural)%></legend>
+<fieldset class="box" id="project_trackers"><legend><%=l(:label_tracker_plural)%></legend>
 <% @trackers.each do |tracker| %>
     <label class="floating">
     <%= check_box_tag 'project[tracker_ids][]', tracker.id, @project.trackers.include?(tracker) %>
@@ -36,7 +50,7 @@
 <% end %>
 
 <% unless @issue_custom_fields.empty? %>
-<fieldset class="box"><legend><%=l(:label_custom_field_plural)%></legend>
+<fieldset class="box" id="project_issue_custom_fields"><legend><%=l(:label_custom_field_plural)%></legend>
 <% @issue_custom_fields.each do |custom_field| %>
     <label class="floating">
 	<%= check_box_tag 'project[issue_custom_field_ids][]', custom_field.id, (@project.all_issue_custom_fields.include? custom_field), (custom_field.is_for_all? ? {:disabled => "disabled"} : {}) %>
@@ -46,4 +60,5 @@
 <%= hidden_field_tag 'project[issue_custom_field_ids][]', '' %>
 </fieldset>
 <% end %>
+<% end %>
 <!--[eoform:project]-->

app/views/projects/new.html.erb

@@ -2,18 +2,6 @@
 
 <% labelled_tabular_form_for :project, @project, :url => { :action => "create" } do |f| %>
 <%= render :partial => 'form', :locals => { :f => f } %>
-
-<fieldset class="box"><legend><%= l(:label_module_plural) %></legend>
-<% Redmine::AccessControl.available_project_modules.each do |m| %>
-    <label class="floating">
-    <%= check_box_tag 'enabled_modules[]', m, @project.module_enabled?(m) %>
-    <%= l_or_humanize(m, :prefix => "project_module_") %>
-    </label>
-<% end %>
-<%= hidden_field_tag 'enabled_modules[]', '' %>
-
-</fieldset>
-
 <%= submit_tag l(:button_save) %>
 <%= javascript_tag "Form.Element.focus('project_name');" %>
 <% end %>

app/views/projects/settings/_modules.rhtml

@@ -7,7 +7,7 @@
 <legend><%= l(:text_select_project_modules) %></legend>
 
 <% Redmine::AccessControl.available_project_modules.each do |m| %>
-<p><label><%= check_box_tag 'enabled_modules[]', m, @project.module_enabled?(m) -%>
+<p><label><%= check_box_tag 'enabled_module_names[]', m, @project.module_enabled?(m) -%>
  <%= l_or_humanize(m, :prefix => "project_module_") %></label></p>
 <% end %>
 </fieldset>

app/views/wiki/history.rhtml

@@ -4,7 +4,7 @@
 
 <% form_tag({:action => "diff"}, :method => :get) do %>
   <%= hidden_field_tag('project_id', h(@project.to_param)) %>
-<table class="list">
+<table class="list wiki-page-versions">
 <thead><tr>
     <th>#</th>
     <th></th>
@@ -18,14 +18,14 @@
 <% show_diff = @versions.size > 1 %>
 <% line_num = 1 %>
 <% @versions.each do |ver| %>
-<tr class="<%= cycle("odd", "even") %>">
+<tr class="wiki-page-version <%= cycle("odd", "even") %>">
     <td class="id"><%= link_to ver.version, :action => 'show', :id => @page.title, :project_id => @page.project, :version => ver.version %></td>
     <td class="checkbox"><%= radio_button_tag('version', ver.version, (line_num==1), :id => "cb-#{line_num}", :onclick => "$('cbto-#{line_num+1}').checked=true;") if show_diff && (line_num < @versions.size) %></td>
     <td class="checkbox"><%= radio_button_tag('version_from', ver.version, (line_num==2), :id => "cbto-#{line_num}") if show_diff && (line_num > 1) %></td>
-    <td align="center"><%= format_time(ver.updated_on) %></td>
+    <td class="updated_on"><%= format_time(ver.updated_on) %></td>
-    <td><%= link_to_user ver.author %></td>
+    <td class="author"><%= link_to_user ver.author %></td>
-    <td><%=h ver.comments %></td>
+    <td class="comments"><%=h ver.comments %></td>
-    <td align="center"><%= link_to l(:button_annotate), :action => 'annotate', :id => @page.title, :version => ver.version %></td>
+    <td class="buttons"><%= link_to l(:button_annotate), :action => 'annotate', :id => @page.title, :version => ver.version %></td>
 </tr>
 <% line_num += 1 %>
 <% end %>

public/javascripts/application.js

@@ -232,6 +232,24 @@ function observeRelatedIssueField(url) {
                            });
 }
 
+function setVisible(id, visible) {
+  var el = $(id);
+  if (el) {if (visible) {el.show();} else {el.hide();}}
+}
+
+function observeProjectModules() {
+  var f = function() {
+    /* Hides trackers and issues custom fields on the new project form when issue_tracking module is disabled */
+    var c = ($('project_enabled_module_names_issue_tracking').checked == true);
+    setVisible('project_trackers', c);
+    setVisible('project_issue_custom_fields', c);
+  };
+  
+  Event.observe(window, 'load', f);
+  Event.observe('project_enabled_module_names_issue_tracking', 'change', f);
+}
+
+
 /* shows and hides ajax indicator */
 Ajax.Responders.register({
     onCreate: function(){

public/stylesheets/application.css

@@ -179,6 +179,8 @@ tr.user td { white-space: nowrap; }
 tr.user.locked, tr.user.registered { color: #aaa; }
 tr.user.locked a, tr.user.registered a { color: #aaa; }
 
+tr.wiki-page-version td.updated_on, tr.wiki-page-version td.author {text-align:center;}
+
 tr.time-entry { text-align: center; white-space: nowrap; }
 tr.time-entry td.subject, tr.time-entry td.comments { text-align: left; white-space: normal; }
 td.hours { text-align: right; font-weight: bold; padding-right: 0.5em; }

test/fixtures/roles.yml

@@ -7,6 +7,7 @@ roles_001:
     --- 
     - :add_project
     - :edit_project
+    - :select_project_modules
     - :manage_members
     - :manage_versions
     - :manage_categories

test/functional/projects_controller_test.rb

@@ -154,7 +154,8 @@ class ProjectsControllerTest < ActionController::TestCase
             :custom_field_values => { '3' => 'Beta' },
             :tracker_ids => ['1', '3'],
             # an issue custom field that is not for all project
-            :issue_custom_field_ids => ['9']
+            :issue_custom_field_ids => ['9'],
+            :enabled_module_names => ['issue_tracking', 'news', 'repository']
           }
         assert_redirected_to '/projects/blog/settings'
         
@@ -167,6 +168,7 @@ class ProjectsControllerTest < ActionController::TestCase
         assert_nil project.parent
         assert_equal 'Beta', project.custom_value_for(3).value
         assert_equal [1, 3], project.trackers.map(&:id).sort
+        assert_equal ['issue_tracking', 'news', 'repository'], project.enabled_module_names.sort
         assert project.issue_custom_fields.include?(IssueCustomField.find(9))
       end
       
@@ -197,7 +199,9 @@ class ProjectsControllerTest < ActionController::TestCase
                                  :description => "weblog",
                                  :identifier => "blog",
                                  :is_public => 1,
-                                 :custom_field_values => { '3' => 'Beta' }
+                                 :custom_field_values => { '3' => 'Beta' },
+                                 :tracker_ids => ['1', '3'],
+                                 :enabled_module_names => ['issue_tracking', 'news', 'repository']
                                 }
         
         assert_redirected_to '/projects/blog/settings'
@@ -206,6 +210,8 @@ class ProjectsControllerTest < ActionController::TestCase
         assert_kind_of Project, project
         assert_equal 'weblog', project.description 
         assert_equal true, project.is_public?
+        assert_equal [1, 3], project.trackers.map(&:id).sort
+        assert_equal ['issue_tracking', 'news', 'repository'], project.enabled_module_names.sort
         
         # User should be added as a project member
         assert User.find(9).member_of?(project)
@@ -282,6 +288,12 @@ class ProjectsControllerTest < ActionController::TestCase
     end
   end
   
+  def test_create_should_not_accept_get
+    @request.session[:user_id] = 1
+    get :create
+    assert_response :method_not_allowed
+  end
+  
   def test_show_by_id
     get :show, :id => 1
     assert_response :success
@@ -360,6 +372,21 @@ class ProjectsControllerTest < ActionController::TestCase
     assert_equal 'Test changed name', project.name
   end
 
+  def test_modules
+    @request.session[:user_id] = 2
+    Project.find(1).enabled_module_names = ['issue_tracking', 'news']
+    
+    post :modules, :id => 1, :enabled_module_names => ['issue_tracking', 'repository', 'documents']
+    assert_redirected_to '/projects/ecookbook/settings/modules'
+    assert_equal ['documents', 'issue_tracking', 'repository'], Project.find(1).enabled_module_names.sort
+  end
+
+  def test_modules_should_not_allow_get
+    @request.session[:user_id] = 1
+    get :modules, :id => 1
+    assert_response :method_not_allowed
+  end
+  
   def test_get_destroy
     @request.session[:user_id] = 1 # admin
     get :destroy, :id => 1

test/integration/api_test/projects_test.rb

@@ -122,12 +122,35 @@ class ApiTest::ProjectsTest < ActionController::IntegrationTest
           project = Project.first(:order => 'id DESC')
           assert_equal 'API test', project.name
           assert_equal 'api-test', project.identifier
-          assert_equal ['issue_tracking', 'repository'], project.enabled_module_names
+          assert_equal ['issue_tracking', 'repository'], project.enabled_module_names.sort
+          assert_equal Tracker.all.size, project.trackers.size
       
           assert_response :created
           assert_equal 'application/xml', @response.content_type
           assert_tag 'project', :child => {:tag => 'id', :content => project.id.to_s}
         end
+        
+        should "accept enabled_module_names attribute" do
+          @parameters[:project].merge!({:enabled_module_names => ['issue_tracking', 'news', 'time_tracking']})
+          
+          assert_difference('Project.count') do
+            post '/projects.xml', @parameters, :authorization => credentials('admin')
+          end
+          
+          project = Project.first(:order => 'id DESC')
+          assert_equal ['issue_tracking', 'news', 'time_tracking'], project.enabled_module_names.sort
+        end
+        
+        should "accept tracker_ids attribute" do
+          @parameters[:project].merge!({:tracker_ids => [1, 3]})
+          
+          assert_difference('Project.count') do
+            post '/projects.xml', @parameters, :authorization => credentials('admin')
+          end
+          
+          project = Project.first(:order => 'id DESC')
+          assert_equal [1, 3], project.trackers.map(&:id).sort
+        end
       end
     end
     
@@ -171,6 +194,28 @@ class ApiTest::ProjectsTest < ActionController::IntegrationTest
           project = Project.find(2)
           assert_equal 'API update', project.name
         end
+        
+        should "accept enabled_module_names attribute" do
+          @parameters[:project].merge!({:enabled_module_names => ['issue_tracking', 'news', 'time_tracking']})
+          
+          assert_no_difference 'Project.count' do
+            put '/projects/2.xml', @parameters, :authorization => credentials('admin')
+          end
+          assert_response :ok
+          project = Project.find(2)
+          assert_equal ['issue_tracking', 'news', 'time_tracking'], project.enabled_module_names.sort
+        end
+        
+        should "accept tracker_ids attribute" do
+          @parameters[:project].merge!({:tracker_ids => [1, 3]})
+          
+          assert_no_difference 'Project.count' do
+            put '/projects/2.xml', @parameters, :authorization => credentials('admin')
+          end
+          assert_response :ok
+          project = Project.find(2)
+          assert_equal [1, 3], project.trackers.map(&:id).sort
+        end
       end
     end
     

test/unit/project_nested_set_test.rb

@@ -19,40 +19,100 @@ require File.expand_path('../../test_helper', __FILE__)
 
 class ProjectNestedSetTest < ActiveSupport::TestCase
   
-  def setup
+  context "nested set" do
-    Project.delete_all
+    setup do
-  end
+      Project.delete_all
 
-  def test_destroy_root_and_chldren_should_not_mess_up_the_tree
+      @a = Project.create!(:name => 'Project A', :identifier => 'projecta')
-    a = Project.create!(:name => 'Project A', :identifier => 'projecta')
+      @a1 = Project.create!(:name => 'Project A1', :identifier => 'projecta1')
-    a1 = Project.create!(:name => 'Project A1', :identifier => 'projecta1')
+      @a1.set_parent!(@a)
-    a2 = Project.create!(:name => 'Project A2', :identifier => 'projecta2')
+      @a2 = Project.create!(:name => 'Project A2', :identifier => 'projecta2')
-    a1.set_parent!(a)
+      @a2.set_parent!(@a)
-    a2.set_parent!(a)
-    b = Project.create!(:name => 'Project B', :identifier => 'projectb')
-    b1 = Project.create!(:name => 'Project B1', :identifier => 'projectb1')
-    b1.set_parent!(b)
       
-    a.reload
+      @b = Project.create!(:name => 'Project B', :identifier => 'projectb')
-    a1.reload
+      @b1 = Project.create!(:name => 'Project B1', :identifier => 'projectb1')
-    a2.reload
+      @b1.set_parent!(@b)
-    b.reload
+      @b11 = Project.create!(:name => 'Project B11', :identifier => 'projectb11')
-    b1.reload
+      @b11.set_parent!(@b1)
+      @b2 = Project.create!(:name => 'Project B2', :identifier => 'projectb2')
+      @b2.set_parent!(@b)
       
-    assert_equal [nil, 1, 6], [a.parent_id, a.lft, a.rgt]
+      @c = Project.create!(:name => 'Project C', :identifier => 'projectc')
-    assert_equal [a.id, 2, 3], [a1.parent_id, a1.lft, a1.rgt]
+      @c1 = Project.create!(:name => 'Project C1', :identifier => 'projectc1')
-    assert_equal [a.id, 4, 5], [a2.parent_id, a2.lft, a2.rgt]
+      @c1.set_parent!(@c)
-    assert_equal [nil, 7, 10], [b.parent_id, b.lft, b.rgt]
-    assert_equal [b.id, 8, 9], [b1.parent_id, b1.lft, b1.rgt]
       
-    assert_difference 'Project.count', -3 do
+      [@a, @a1, @a2, @b, @b1, @b11, @b2, @c, @c1].each(&:reload)
-      a.destroy
     end
     
-    b.reload
+    context "#create" do
-    b1.reload
+      should "build valid tree" do
+        assert_nested_set_values({
+          @a   => [nil,   1,  6],
+          @a1  => [@a.id, 2,  3],
+          @a2  => [@a.id, 4,  5],
+          @b   => [nil,   7, 14],
+          @b1  => [@b.id, 8, 11],
+          @b11 => [@b1.id,9, 10],
+          @b2  => [@b.id,12, 13],
+          @c   => [nil,  15, 18],
+          @c1  => [@c.id,16, 17]
+        })
+      end
+    end
     
-    assert_equal [nil, 1, 4], [b.parent_id, b.lft, b.rgt]
+    context "#set_parent!" do
-    assert_equal [b.id, 2, 3], [b1.parent_id, b1.lft, b1.rgt]
+      should "keep valid tree" do
+        assert_no_difference 'Project.count' do
+          Project.find_by_name('Project B1').set_parent!(Project.find_by_name('Project A2'))
+        end
+        assert_nested_set_values({
+          @a   => [nil,   1, 10],
+          @a2  => [@a.id, 4,  9],
+          @b1  => [@a2.id,5,  8],
+          @b11 => [@b1.id,6,  7],
+          @b   => [nil,  11, 14],
+          @c   => [nil,  15, 18]
+        })
+      end
+    end
+  
+    context "#destroy" do
+      context "a root with children" do
+        should "not mess up the tree" do
+          assert_difference 'Project.count', -4 do
+            Project.find_by_name('Project B').destroy
+          end
+          assert_nested_set_values({
+            @a  => [nil,   1,  6],
+            @a1 => [@a.id, 2,  3],
+            @a2 => [@a.id, 4,  5],
+            @c  => [nil,   7, 10],
+            @c1 => [@c.id, 8,  9]
+          })
+        end
+      end
+      
+      context "a child with children" do
+        should "not mess up the tree" do
+          assert_difference 'Project.count', -2 do
+            Project.find_by_name('Project B1').destroy
+          end
+          assert_nested_set_values({
+            @a  => [nil,   1,  6],
+            @b  => [nil,   7, 10],
+            @b2 => [@b.id, 8,  9],
+            @c  => [nil,  11, 14]
+          })
+        end
+      end
+    end
+  end
+  
+  def assert_nested_set_values(h)
+    assert Project.valid?
+    h.each do |project, expected|
+      project.reload
+      assert_equal expected, [project.parent_id, project.lft, project.rgt], "Unexpected nested set values for #{project.name}"
+    end
   end
 end