refactors work package controller to always use work_package method

app/controllers/work_packages_controller.rb

@@ -12,8 +12,6 @@
 class WorkPackagesController < ApplicationController
   unloadable
 
-  helper :timelines, :planning_elements
-
   include ExtendedHTTP
   include Redmine::Export::PDF
 
@@ -35,17 +33,9 @@ class WorkPackagesController < ApplicationController
   model_object WorkPackage
 
   before_filter :disable_api
-  before_filter :find_model_object_and_project, :only => [:show, :edit, :update]
-  before_filter :find_project_by_project_id, :only => [:new, :create]
-  before_filter :not_found_unless_work_package, :only => :new_type
-  before_filter :project, :only => [:new_type]
-  before_filter :authorize,
-                :assign_planning_elements
-  before_filter :apply_at_timestamp, :only => [:show]
-
-
-  helper :timelines
-  helper :timelines_journals
+  before_filter :not_found_unless_work_package,
+                :project,
+                :authorize
 
   def show
     respond_to do |format|
@@ -179,14 +169,19 @@ class WorkPackagesController < ApplicationController
 
   def new_work_package
     @new_work_package ||= begin
-      params[:work_package] ||= {}
-      sti_type = params[:sti_type] || params[:work_package][:sti_type] || 'Issue'
-
       project = find_project_by_project_id
-      permitted = permitted_params.new_work_package(:project => project)
+
+      permitted = if params[:work_package]
+                    permitted_params.new_work_package(:project => project)
+                  else
+                    params[:work_package] ||= {}
+                    {}
+                  end
 
       permitted[:author] = current_user
 
+      sti_type = params[:sti_type] || params[:work_package][:sti_type] || 'Issue'
+
       wp = case sti_type
            when PlanningElement.to_s
              project.add_planning_element(permitted)
@@ -307,19 +302,4 @@ class WorkPackagesController < ApplicationController
   def send_notifications?
     params[:send_notification] == '0' ? false : true
   end
-
-  def assign_planning_elements
-    @planning_elements = @project.planning_elements.without_deleted
-  end
-
-  def apply_at_timestamp
-    return if params[:at].blank?
-
-    time = Time.at(Integer(params[:at]))
-    # intentionally rebuilding scope chain to avoid without_deleted scope
-    @planning_elements = @project.planning_elements.at_time(time)
-
-  rescue ArgumentError
-    render_errors(:at => 'unknown format')
-  end
 end

app/models/permitted_params.rb

@@ -84,7 +84,7 @@ class PermittedParams < Struct.new(:params, :user)
   def new_work_package(args = {})
     permitted = permitted_attributes(:new_work_package, args)
 
-    permitted_params = params[:work_package].permit(*permitted)
+    permitted_params = params.require(:work_package).permit(*permitted)
 
     permitted_params.merge!(custom_field_values(:work_package))
 

app/views/work_packages/_edit.html.erb

@@ -21,7 +21,7 @@ See doc/COPYRIGHT.rdoc for more details.
                                         :multipart => true
                                        } do |f| %>
 
-  <%= error_messages_for 'work_package' %>
+  <%= error_messages_for work_package %>
 
   <div class="box">
 

features/work_packages/editable_fields.feature

@@ -25,6 +25,7 @@ Feature: Fields editable on work package edit
       | name | prio1 |
     And the role "manager" may have the following rights:
       | edit_work_packages |
+      | view_work_packages |
       | manage_subtasks    |
     And the project "ecookbook" has 1 version with:
       | name | version1 |
@@ -57,6 +58,7 @@ Feature: Fields editable on work package edit
   Scenario: Going to the page and viewing timelog fields if this module is enabled
     Given the role "manager" may have the following rights:
       | edit_work_packages |
+      | view_work_packages |
       | log_time           |
 
     And there are the following planning elements in project "ecookbook":

features/work_packages/error_on_update.feature

@@ -10,7 +10,8 @@
 # See doc/COPYRIGHT.rdoc for more details.
 #++
 
-Feature: Logging time on work package update
+Feature: Error messages are displayed
+
   Background:
     Given there is 1 user with:
       | login     | manager |
@@ -33,7 +34,7 @@ Feature: Logging time on work package update
     And I am already logged in as "manager"
 
   @javascript
-  Scenario: Logging time
+  Scenario: Inserting a blank subject results in an error beeing shown
     When I go to the edit page of the work package called "pe1"
      And I follow "More"
      And I fill in the following:

features/work_packages/navigate_to_edit.feature

@@ -6,6 +6,7 @@ Feature: Navigating to the work package edit page
     And there is a role "manager"
     And the role "manager" may have the following rights:
       | edit_work_packages |
+      | view_work_packages |
 
     And there is 1 project with the following:
       | identifier | ecookbook |

spec/controllers/work_packages_controller_spec.rb

@@ -592,12 +592,17 @@ describe WorkPackagesController do
   end
 
   describe :new_work_package do
+    let(:wp_params) { { :wp_attribute => double('wp_attribute') } }
+
     describe 'when the type is "PlanningElement"' do
       before do
         controller.params = { :sti_type => 'PlanningElement',
                               :work_package => {} }
-        controller.stub!(:find_project_by_project_id).and_return(project)
-        controller.stub!(:current_user).and_return(stub_user)
+        controller.stub(:find_project_by_project_id).and_return(project)
+        controller.stub(:current_user).and_return(stub_user)
+        controller.send(:permitted_params).should_receive(:new_work_package)
+                                          .with(:project => project)
+                                          .and_return(wp_params)
 
         project.should_receive(:add_planning_element) do |args|
 
@@ -623,8 +628,11 @@ describe WorkPackagesController do
         controller.params = { :sti_type => 'Issue',
                               :work_package => {} }
 
-        controller.stub!(:find_project_by_project_id).and_return(project)
-        controller.stub!(:current_user).and_return(stub_user)
+        controller.stub(:find_project_by_project_id).and_return(project)
+        controller.stub(:current_user).and_return(stub_user)
+        controller.send(:permitted_params).should_receive(:new_work_package)
+                                          .with(:project => project)
+                                          .and_return(wp_params)
 
         project.should_receive(:add_issue) do |args|