Merge pull request #304 from opf/fix/type_switching

pull/307/head
Hagen Schink 11 years ago
commit e3fb94dfd2
  1. 137
      app/controllers/work_packages_controller.rb
  2. 48
      app/models/permitted_params.rb
  3. 6
      app/models/project.rb
  4. 18
      app/models/work_package.rb
  5. 2
      app/views/work_packages/_edit.html.erb
  6. 1
      doc/CHANGELOG.md
  7. 2
      features/work_packages/editable_fields.feature
  8. 5
      features/work_packages/error_on_update.feature
  9. 1
      features/work_packages/navigate_to_edit.feature
  10. 75
      features/work_packages/switch_type.feature
  11. 639
      spec/controllers/work_packages_controller_spec.rb
  12. 49
      spec/models/permitted_params_spec.rb
  13. 26
      spec/models/project_spec.rb
  14. 18
      spec/models/work_package_spec.rb
  15. 21
      spec/permissions/add_work_packages_spec.rb
  16. 21
      spec/permissions/edit_work_packages_spec.rb
  17. 19
      spec/permissions/view_work_packages_spec.rb
  18. 50
      spec/support/permission_specs.rb
  19. 29
      spec/support/shared/become_member.rb

@ -12,14 +12,11 @@
class WorkPackagesController < ApplicationController
unloadable
helper :timelines, :planning_elements
include ExtendedHTTP
include Redmine::Export::PDF
current_menu_item do |controller|
begin
wp = controller.new_work_package || controller.work_package
wp = controller.work_package
case wp
when PlanningElement
@ -32,19 +29,10 @@ class WorkPackagesController < ApplicationController
end
end
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 :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|
@ -84,7 +72,7 @@ class WorkPackagesController < ApplicationController
def new
respond_to do |format|
format.html { render :locals => { :work_package => new_work_package,
format.html { render :locals => { :work_package => work_package,
:project => project,
:priorities => priorities,
:user => current_user } }
@ -92,8 +80,11 @@ class WorkPackagesController < ApplicationController
end
def new_type
safe_params = permitted_params.update_work_package(:project => project)
work_package.update_by(current_user, safe_params)
respond_to do |format|
format.js { render :locals => { :work_package => work_package || new_work_package,
format.js { render :locals => { :work_package => work_package,
:project => project,
:priorities => priorities,
:user => current_user } }
@ -101,19 +92,19 @@ class WorkPackagesController < ApplicationController
end
def create
call_hook(:controller_work_package_new_before_save, { :params => params, :work_package => new_work_package })
call_hook(:controller_work_package_new_before_save, { :params => params, :work_package => work_package })
WorkPackageObserver.instance.send_notification = send_notifications?
if new_work_package.save
if work_package.save
flash[:notice] = I18n.t(:notice_successful_create)
Attachment.attach_files(new_work_package, params[:attachments])
render_attachment_warning_if_needed(new_work_package)
Attachment.attach_files(work_package, params[:attachments])
render_attachment_warning_if_needed(work_package)
call_hook(:controller_work_pacakge_new_after_save, { :params => params, :work_package => new_work_package })
call_hook(:controller_work_pacakge_new_after_save, { :params => params, :work_package => work_package })
redirect_to(work_package_path(new_work_package))
redirect_to(work_package_path(work_package))
else
respond_to do |format|
format.html { render :action => 'new' }
@ -138,7 +129,7 @@ class WorkPackagesController < ApplicationController
configure_update_notification(send_notifications?)
safe_params = permitted_params.update_work_package(:project => project)
updated = work_package.update_by(current_user, safe_params)
updated = work_package.update_by!(current_user, safe_params)
render_attachment_warning_if_needed(work_package)
@ -152,8 +143,17 @@ class WorkPackagesController < ApplicationController
end
end
def work_package
@work_package ||= begin
if params[:id]
existing_work_package
elsif params[:project_id]
new_work_package
end
end
def existing_work_package
@existing_work_package ||= begin
wp = WorkPackage.includes(:project)
.find_by_id(params[:id])
@ -166,13 +166,20 @@ 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 = Project.find_visible(current_user, params[:project_id])
return nil unless project
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)
@ -189,11 +196,7 @@ class WorkPackagesController < ApplicationController
end
def project
@project ||= if params[:project_id]
find_project_by_project_id
elsif work_package
work_package.project
end
@project ||= work_package.project
end
def journals
@ -203,48 +206,21 @@ class WorkPackagesController < ApplicationController
end
def ancestors
@ancestors ||= begin
case work_package
when PlanningElement
# Right now all planning_elements of a tree are part of the same project.
# That means that a user can either see all planning_elements or none.
# Thus, after access to a planning element is established (work_package) we
# currently need no extra check for the ancestors/descendants
work_package.ancestors
when Issue
work_package.ancestors.visible.includes(:type,
@ancestors ||= work_package.ancestors.visible.includes(:type,
:assigned_to,
:status,
:priority,
:fixed_version,
:project)
end
def descendants
@descendants ||= work_package.descendants.visible.includes(:type,
:assigned_to,
:status,
:priority,
:fixed_version,
:project)
else
[]
end
end
end
def descendants
@descendants ||= begin
case work_package
when PlanningElement
# Right now all planning_elements of a tree are part of the same project.
# That means that a user can either see all planning_elements or none.
# Thus, after access to a planning element is established (work_package) we
# currently need no extra check for the ancestors/descendants
work_package.descendants
when Issue
work_package.descendants.visible.includes(:type,
:assigned_to,
:status,
:priority,
:fixed_version,
:project)
else
[]
end
end
end
@ -286,6 +262,10 @@ class WorkPackagesController < ApplicationController
protected
def not_found_unless_work_package
render_404 unless work_package
end
def configure_update_notification(state = true)
JournalObserver.instance.send_notification = state
end
@ -293,19 +273,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

@ -84,28 +84,30 @@ class PermittedParams < Struct.new(:params, :user)
def new_work_package(args = {})
permitted = permitted_attributes(:new_work_package, args)
params[:work_package].permit(*permitted)
end
permitted_params = params.require(:work_package).permit(*permitted)
def update_work_package(args = {})
permitted = permitted_attributes(:new_work_package, args)
permitted_params.merge!(custom_field_values(:work_package))
params[:work_package].permit(*permitted)
permitted_params
end
alias :update_work_package :new_work_package
def user_update_as_admin
if user.admin?
params.require(:user).permit(:firstname,
:lastname,
:mail,
:mail_notification,
:language,
:custom_field_values,
:custom_fields,
:identity_url,
:auth_source_id,
:force_password_change,
:group_ids => [])
permitted_params = params.require(:user).permit(:firstname,
:lastname,
:mail,
:mail_notification,
:language,
:custom_fields,
:identity_url,
:auth_source_id,
:force_password_change,
:group_ids => [])
permitted_params.merge!(custom_field_values(:user))
permitted_params
else
params.require(:user).permit()
end
@ -134,6 +136,20 @@ class PermittedParams < Struct.new(:params, :user)
protected
def custom_field_values(key)
# a hash of arbitrary values is not supported by strong params
# thus we do it by hand
values = params.require(key)[:custom_field_values] || {}
# only permit values following the schema
# 'id as string' => 'value as string'
values.reject!{ |k, v| k.to_i < 1 || !v.is_a?(String) }
values.empty? ?
{} :
{ "custom_field_values" => values }
end
def permitted_attributes(key, additions = {})
merged_args = { :params => params, :user => user }.merge(additions)

@ -410,6 +410,12 @@ class Project < ActiveRecord::Base
end
end
def self.find_visible(user, *args)
with_scope(:find => where(Project.visible_by(user))) do
find(*args)
end
end
def to_param
# id is used for projects with a numeric identifier (compatibility)
@to_param ||= (identifier.to_s =~ %r{^\d*$} ? id : identifier)

@ -313,13 +313,12 @@ class WorkPackage < ActiveRecord::Base
# TODO: move into Business Object and rename to update
# update for now is a private method defined by AR
def update_by(user, attributes)
init_journal(user, attributes.delete(:notes)) if attributes[:notes]
def update_by!(user, attributes)
raw_attachments = attributes.delete(:attachments)
add_time_entry_for(user, attributes.delete(:time_entry))
if update_attributes(attributes)
update_by(user, attributes)
if save
# as attach_files always saves an attachment right away
# it is not possible to stage attaching and check for
# valid. If this would be possible, we could check
@ -328,6 +327,15 @@ class WorkPackage < ActiveRecord::Base
end
end
def update_by(user, attributes)
init_journal(user, attributes.delete(:notes)) if attributes[:notes]
add_time_entry_for(user, attributes.delete(:time_entry))
attributes.delete(:attachments)
self.attributes = attributes
end
def is_milestone?
type && type.is_milestone?
end

@ -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">

@ -1,6 +1,7 @@
# Changelog
* `#1541` Use Rails 3.2.14 instead of Git Branch
* `#1598` Switching type of work package looses inserted data
## 3.0.0pre10

@ -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":

@ -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:

@ -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 |

@ -0,0 +1,75 @@
#-- copyright
# OpenProject is a project management system.
#
# Copyright (C) 2012-2013 the OpenProject Team
#
# This program is free software; you can redistribute it and/or
# modify it under the terms of the GNU General Public License version 3.
#
# See doc/COPYRIGHT.rdoc for more details.
#++
Feature: Switching types of work packages
Background:
Given there is 1 project with the following:
| name | project1 |
| identifier | project1 |
And I am working in project "project1"
And the project "project1" has the following types:
| name | position |
| Bug | 1 |
| Feature | 2 |
And there is a default issuepriority with:
| name | Normal |
And there is a role "member"
And the role "member" may have the following rights:
| view_work_packages |
| edit_work_packages |
And there is 1 user with the following:
| login | bob |
| firstname | Bob |
| lastname | Bobbit |
And the user "bob" is a "member" in the project "project1"
Given the user "bob" has 1 issue with the following:
| subject | wp1 |
| description | Initial description |
| type | Bug |
And I am already logged in as "bob"
@javascript
Scenario: Switching type should keep the inserted value
When I go to the edit page of the work package "wp1"
And I fill in the following:
| Responsible | Bob Bobbit |
And I follow "More"
And I select "Feature" from "Type"
Then I should be on the edit page of the work package "wp1"
And I should see the following fields:
| Responsible | Bob Bobbit |
@javascript
Scenario: Switching type should update the presented custom fields
Given the following work package custom fields are defined:
| name | type |
| cfBug | int |
| cfFeature | int |
| cfAll | int |
And the custom field "cfBug" is activated for type "Bug"
And the custom field "cfFeature" is activated for type "Feature"
And the custom field "cfAll" is activated for type "Bug"
And the custom field "cfAll" is activated for type "Feature"
When I go to the edit page of the work package "wp1"
And I follow "More"
And I fill in the following:
| cfAll | 5 |
And I select "Feature" from "Type"
Then I should be on the edit page of the work package "wp1"
And I should see the following fields:
| cfFeature | |
| cfAll | 5 |

@ -63,153 +63,78 @@ describe WorkPackagesController do
let(:stub_project) { FactoryGirl.build_stubbed(:project, :identifier => 'test_project', :is_public => false) }
let(:stub_issue) { FactoryGirl.build_stubbed(:issue, :project_id => stub_project.id) }
let(:stub_user) { FactoryGirl.build_stubbed(:user) }
let(:stub_work_package) { double("work_package", :id => 1337, :project => stub_project).as_null_object }
let(:current_user) { FactoryGirl.create(:user) }
describe 'show.html' do
become_admin
describe 'w/o a valid planning element id' do
describe 'w/o being a member or administrator' do
become_non_member
it 'renders a 404 page' do
get 'show', :id => '1337'
def self.requires_permission_in_project(&block)
describe 'w/o the permission to see the project/work_package' do
before do
controller.stub(:work_package).and_return(nil)
response.response_code.should === 404
end
call_action
end
describe 'w/ the current user being a member' do
become_member_with_view_planning_element_permissions
it 'raises ActiveRecord::RecordNotFound errors' do
get 'show', :id => '1337'
response.response_code.should === 404
end
it 'should render a 404' do
response.response_code.should === 404
end
end
describe 'w/ a valid planning element id' do
become_admin
describe 'w/o being a member or administrator' do
become_non_member
it 'renders a 403 Forbidden page' do
get 'show', :id => planning_element.id
describe 'w/ the permission to see the project
w/ having the necessary permissions' do
response.response_code.should == 403
end
before do
controller.stub(:work_package).and_return(stub_work_package)
controller.should_receive(:authorize).and_return(true)
end
describe 'w/ the current user being a member' do
become_member_with_view_planning_element_permissions
before do
get 'show', :id => planning_element.id
end
it 'renders the show builder template' do
response.should render_template('work_packages/show', :formats => ["html"], :layout => :base)
end
end
instance_eval(&block)
end
end
describe 'show.pdf' do
become_admin
describe 'w/o a valid planning element id' do
describe 'w/o being a member or administrator' do
become_non_member
it 'renders a 404 page' do
get 'show', :format => 'pdf',
:id => '1337'
response.response_code.should === 404
end
end
describe 'w/ the current user being a member' do
become_member_with_view_planning_element_permissions
describe 'show.html' do
let(:call_action) { get('show', :id => '1337') }
it 'raises ActiveRecord::RecordNotFound errors' do
get 'show', :format => 'pdf',
:id => '1337'
requires_permission_in_project do
it 'renders the show builder template' do
call_action
response.response_code.should === 404
end
response.should render_template('work_packages/show', :formats => ["html"],
:layout => :base)
end
end
end
describe 'w/ a valid planning element id' do
become_admin
describe 'w/o being a member or administrator' do
become_non_member
it 'renders a 403 Forbidden page' do
get 'show', :format => 'pdf',
:id => planning_element.id
response.response_code.should == 403
end
end
describe 'w/ the current user being a member' do
become_member_with_view_planning_element_permissions
describe 'show.pdf' do
let(:call_action) { get('show', :format => 'pdf', :id => '1337') }
it "should respond with a pdf" do
pdf = double('pdf')
requires_permission_in_project do
it 'respond with a pdf' do
pdf = double('pdf')
expected_name = "#{planning_element.project.identifier}-#{planning_element.id}.pdf"
controller.stub!(:issue_to_pdf).and_return(pdf)
controller.should_receive(:send_data).with(pdf,
:type => 'application/pdf',
:filename => expected_name).and_call_original
get 'show', :format => 'pdf',
:id => planning_element.id
end
expected_name = "#{stub_work_package.project.identifier}-#{stub_work_package.id}.pdf"
controller.stub!(:issue_to_pdf).and_return(pdf)
controller.should_receive(:send_data).with(pdf,
:type => 'application/pdf',
:filename => expected_name).and_call_original
call_action
end
end
end
describe 'new.html' do
describe 'w/o specifying a project_id' do
before do
get 'new'
end
it 'should return 404 Not found' do
response.response_code.should == 404
end
end
describe 'w/o being a member' do
before do
get 'new', :project_id => project.id
end
it 'should return 403 Forbidden' do
response.response_code.should == 403
end
end
describe 'w/ beeing a member
w/ having the necessary permissions' do
become_member_with_permissions [:add_work_packages]
describe 'new.html' do
let(:call_action) { get('new', :format => 'html', :project_id => 5) }
requires_permission_in_project do
before do
get 'new', :project_id => project.id
call_action
end
it 'renders the new builder template' do
response.should render_template('work_packages/new', :formats => ["html"])
end
@ -217,68 +142,21 @@ describe WorkPackagesController do
response.response_code.should == 200
end
end
describe 'w/ beeing a member
w/o having the necessary permissions' do
become_member_with_permissions []
before do
get 'new', :project_id => project.id
end
it 'should return 403 Forbidden' do
response.response_code.should == 403
end
end
end
describe 'new_type.js' do
describe 'w/o specifying a project_id or an id' do
before do
xhr :get, :new_type
end
it 'should return 403 Not found' do
response.response_code.should == 403
end
end
describe 'w/o being a member' do
before do
xhr :get, :new_type, :project_id => project.id
end
it 'should return 403 Forbidden' do
response.response_code.should == 403
end
end
describe 'w/ beeing a member
w/ having the necessary permissions
w/ specifying a project_id' do
become_member_with_permissions [:add_work_packages]
describe 'new_type.js' do
let(:wp_params) { { :wp_attribute => double('wp_attribute') } }
let(:call_action) { xhr :get, :new_type, :project_id => 5 }
requires_permission_in_project do
before do
xhr :get, :new_type, :project_id => project.id
end
it 'renders the new builder template' do
response.should render_template('work_packages/new_type', :formats => ["html"])
end
it 'should respond with 200 OK' do
response.response_code.should == 200
end
end
describe 'w/ beeing a member
w/ having the necessary permissions
w/ specifying an id' do
become_member_with_permissions [:view_work_packages,
:edit_work_packages]
controller.send(:permitted_params).should_receive(:update_work_package)
.with(:project => stub_project)
.and_return(wp_params)
stub_work_package.should_receive(:update_by).with(current_user, wp_params).and_return(true)
before do
xhr :get, :new_type, :id => planning_element.id
call_action
end
it 'renders the new builder template' do
@ -289,359 +167,280 @@ describe WorkPackagesController do
response.response_code.should == 200
end
end
describe 'w/ beeing a member
w/o having the necessary permissions' do
become_member_with_permissions []
before do
xhr :get, :new_type, :project_id => project.id
end
it 'should return 403 Forbidden' do
response.response_code.should == 403
end
end
end
describe 'create.html' do
describe 'w/o specifying a project_id' do
before do
post 'create'
end
it 'should return 404 Not found' do
response.response_code.should == 404
end
end
describe 'w/o being a member' do
before do
post 'create', :project_id => project.id
end
it 'should return 403 Forbidden' do
response.response_code.should == 403
end
end
describe 'create.html' do
let(:params) { { :project_id => stub_work_package.project.id,
:work_package => { } } }
describe 'w/ beeing a member
w/ having the necessary permissions
w/ having an successful save' do
let(:params) { { :project_id => project.id, :work_package => { } } }
let(:call_action) { post 'create', params }
become_member_with_permissions [:add_work_packages]
requires_permission_in_project do
before do
controller.stub!(:new_work_package).and_return(stub_issue)
stub_issue.should_receive(:save).and_return(true)
end
describe 'w/ having a successful save' do
before do
stub_work_package.should_receive(:save).and_return(true)
end
it 'redirect to show' do
post 'create', params
it 'redirect to show' do
call_action
response.should redirect_to(work_package_path(stub_issue))
end
response.should redirect_to(work_package_path(stub_work_package))
end
it 'should show a flash message' do
disable_flash_sweep
it 'should show a flash message' do
disable_flash_sweep
post 'create', params
call_action
flash[:notice].should == I18n.t(:notice_successful_create)
end
flash[:notice].should == I18n.t(:notice_successful_create)
end
it 'should attach attachments if those are provided' do
params[:attachments] = 'attachment-blubs-data'
it 'should attach attachments if those are provided' do
params[:attachments] = 'attachment-blubs-data'
Attachment.should_receive(:attach_files).with(stub_issue, params[:attachments])
controller.stub!(:render_attachment_warning_if_needed)
Attachment.should_receive(:attach_files).with(stub_work_package, params[:attachments])
controller.stub!(:render_attachment_warning_if_needed)
post 'create', params
call_action
end
end
end
describe 'w/ beeing a member
w/ having the necessary permissions
w/ having an unsuccessful save' do
become_member_with_permissions [:add_work_packages]
describe 'w/ having an unsuccessful save' do
before do
controller.stub!(:new_work_package).and_return(stub_issue)
stub_issue.should_receive(:save).and_return(false)
before do
stub_work_package.should_receive(:save).and_return(false)
post 'create', :project_id => project.id
end
call_action
end
it 'renders the new template' do
response.should render_template('work_packages/new', :formats => ["html"])
it 'renders the new template' do
response.should render_template('work_packages/new', :formats => ["html"])
end
end
end
end
describe 'w/ beeing a member
w/o having the necessary permissions' do
become_member_with_permissions []
describe 'edit.html' do
let(:call_action) { get 'edit', :id => stub_work_package.id }
before do
get 'new', :project_id => project.id
end
requires_permission_in_project do
it 'renders the show builder template' do
call_action
it 'should return 403 Forbidden' do
response.response_code.should == 403
response.should render_template('work_packages/edit', :formats => ["html"], :layout => :base)
end
end
end
describe 'edit.html' do
become_admin
describe 'w/o a valid work_package id' do
describe 'w/o being a member or administrator' do
become_non_member
it 'renders a 404 page' do
get 'edit', :id => '1337'
describe 'update.html' do
let(:wp_params) { { :wp_attribute => double('wp_attribute') } }
let(:params) { { :id => stub_work_package.id, :work_package => wp_params } }
let(:call_action) { put 'update', params }
response.response_code.should === 404
end
requires_permission_in_project do
before do
controller.stub(:work_package).and_return(stub_work_package)
controller.send(:permitted_params).should_receive(:update_work_package)
.with(:project => stub_work_package.project)
.and_return(wp_params)
end
describe 'w/ the current user being a member' do
become_member_with_view_planning_element_permissions
describe 'w/ having a successful save' do
before do
stub_work_package.should_receive(:update_by!)
.with(current_user, wp_params)
.and_return(true)
end
it 'raises ActiveRecord::RecordNotFound errors' do
get 'edit', :id => '1337'
it 'should respond with 200 OK' do
call_action
response.response_code.should === 404
response.response_code.should == 200
end
end
end
describe 'w/ a valid work package id' do
become_admin
describe 'w/o being a member or administrator' do
become_non_member
it 'should show a flash message' do
disable_flash_sweep
it 'renders a 403 Forbidden page' do
get 'edit', :id => planning_element.id
call_action
response.response_code.should == 403
flash[:notice].should == I18n.t(:notice_successful_update)
end
end
describe 'w/ the current user being a member' do
become_member_with_permissions [:edit_work_packages]
describe 'w/ having an unsuccessful save' do
before do
get 'edit', :id => planning_element.id
stub_work_package.should_receive(:update_by!)
.with(current_user, wp_params)
.and_return(false)
end
it 'renders the show builder template' do
it 'render the edit action' do
call_action
response.should render_template('work_packages/edit', :formats => ["html"], :layout => :base)
end
end
end
end
describe 'update.html' do
describe 'w/o being a member' do
before do
put 'update'
end
describe 'w/ having a successful save
w/ having a faulty attachment' do
it 'should return 404 Not Found' do
response.response_code.should == 404
end
end
before do
stub_work_package.should_receive(:update_by!)
.with(current_user, wp_params)
.and_return(true)
stub_work_package.stub(:unsaved_attachments)
.and_return([double('unsaved_attachment')])
end
describe 'w/ beeing a member
w/ having the necessary permissions
w/ a valid wp id
w/ having a successful save' do
let(:wp_params) { { :wp_attribute => double('wp_attribute') } }
let(:params) { { :id => planning_element.id, :work_package => wp_params } }
it 'should respond with 200 OK' do
call_action
become_member_with_permissions [:edit_work_packages]
response.response_code.should == 200
end
before do
controller.stub!(:work_package).and_return(planning_element)
controller.send(:permitted_params).should_receive(:update_work_package)
.with(:project => planning_element.project)
.and_return(wp_params)
planning_element.should_receive(:update_by).with(current_user, wp_params).and_return(true)
end
it 'should show a flash message' do
disable_flash_sweep
it 'should respond with 200 OK' do
put 'update', params
call_action
response.response_code.should == 200
flash[:warning].should == I18n.t(:warning_attachments_not_saved, :count => 1)
end
end
end
end
it 'should show a flash message' do
disable_flash_sweep
put 'update', params
describe :work_package do
describe 'when providing an id (wanting to see an existing wp)' do
describe 'when beeing allowed to see the work_package' do
become_member_with_view_planning_element_permissions
flash[:notice].should == I18n.t(:notice_successful_update)
end
end
it 'should return the work_package' do
controller.params = { id: planning_element.id }
describe 'w/ beeing a member
w/ having the necessary permissions
w/ a valid wp id
w/ having an unsuccessful save' do
let(:wp_params) { { :wp_attribute => double('wp_attribute') } }
let(:params) { { :id => planning_element.id, :work_package => wp_params } }
controller.work_package.should == planning_element
end
become_member_with_permissions [:edit_work_packages]
it 'should return nil for non existing work_packages' do
controller.params = { id: 0 }
before do
controller.stub!(:work_package).and_return(planning_element)
controller.send(:permitted_params).should_receive(:update_work_package)
.with(:project => planning_element.project)
.and_return(wp_params)
planning_element.should_receive(:update_by).with(current_user, wp_params).and_return(false)
controller.work_package.should be_nil
end
end
it 'render the edit action' do
put 'update', params
describe 'when not beeing allowed to see the work_package' do
it 'should return nil' do
controller.params = { id: planning_element.id }
response.should render_template('work_packages/edit', :formats => ["html"], :layout => :base)
controller.work_package.should be_nil
end
end
end
describe 'w/ beeing a member
w/ having the necessary permissions
w/ a valid wp id
w/ having a successful save
w/ having a faulty attachment' do
describe 'when providing a project_id (wanting to build a new wp)' do
let(:wp_params) { { :wp_attribute => double('wp_attribute') } }
let(:params) { { :id => planning_element.id, :work_package => wp_params } }
become_member_with_permissions [:edit_work_packages]
let(:params) { { :project_id => stub_project.id } }
before do
controller.stub!(:work_package).and_return(planning_element)
controller.send(:permitted_params).should_receive(:update_work_package)
.with(:project => planning_element.project)
.and_return(wp_params)
planning_element.should_receive(:update_by).with(current_user, wp_params).and_return(true)
planning_element.stub(:unsaved_attachments).and_return([double('unsaved_attachment')])
Project.stub(:find_visible).and_return stub_project
end
it 'should respond with 200 OK' do
put 'update', params
describe 'when the type is "PlanningElement"' do
before do
controller.params = { :sti_type => 'PlanningElement',
:work_package => {} }.merge(params)
response.response_code.should == 200
end
controller.stub(:current_user).and_return(stub_user)
controller.send(:permitted_params).should_receive(:new_work_package)
.with(:project => stub_project)
.and_return(wp_params)
it 'should show a flash message' do
disable_flash_sweep
stub_project.should_receive(:add_planning_element) do |args|
put 'update', params
expect(args[:author]).to eql stub_user
flash[:warning].should == I18n.t(:warning_attachments_not_saved, :count => 1)
end
end
end
end.and_return(stub_planning_element)
end
describe :work_package do
describe 'when beeing allowed to see the work_package' do
become_member_with_view_planning_element_permissions
it 'should return a new planning element on the project' do
controller.work_package.should == stub_planning_element
end
it 'should return the work_package' do
controller.params = { id: planning_element.id }
it 'should copy over attributes from another work_package provided as the source' do
controller.params[:copy_from] = 2
stub_planning_element.should_receive(:copy_from).with(2, :exclude => [:project_id])
controller.work_package.should == planning_element
controller.work_package
end
end
it 'should return nil for non existing work_packages' do
controller.params = { id: 0 }
controller.work_package.should be_nil
end
end
describe 'when not beeing allowed to see the work_package' do
it 'should return nil' do
controller.params = { id: planning_element.id }
controller.work_package.should be_nil
end
end
end
describe 'when the type is "Issue"' do
before do
controller.params = { :sti_type => 'Issue',
:work_package => {} }.merge(params)
describe :new_work_package do
describe 'when the type is "PlanningElement"' do
before do
controller.params = { :sti_type => 'PlanningElement',
:work_package => {} }
controller.stub!(:project).and_return(project)
controller.stub!(:current_user).and_return(stub_user)
controller.stub(:current_user).and_return(stub_user)
controller.send(:permitted_params).should_receive(:new_work_package)
.with(:project => stub_project)
.and_return(wp_params)
project.should_receive(:add_planning_element) do |args|
stub_project.should_receive(:add_issue) do |args|
expect(args[:author]).to eql stub_user
expect(args[:author]).to eql stub_user
end.and_return(stub_planning_element)
end
end.and_return(stub_issue)
end
it 'should return a new planning element on the project' do
controller.new_work_package.should == stub_planning_element
end
it 'should return a new issue on the project' do
controller.work_package.should == stub_issue
end
it 'should copy over attributes from another work_package provided as the source' do
controller.params[:copy_from] = 2
stub_planning_element.should_receive(:copy_from).with(2, :exclude => [:project_id])
it 'should copy over attributes from another work_package provided as the source' do
controller.params[:copy_from] = 2
stub_issue.should_receive(:copy_from).with(2, :exclude => [:project_id])
controller.new_work_package
controller.work_package
end
end
end
describe 'when the type is "Issue"' do
before do
controller.params = { :sti_type => 'Issue',
:work_package => {} }
controller.stub!(:project).and_return(project)
controller.stub!(:current_user).and_return(stub_user)
project.should_receive(:add_issue) do |args|
expect(args[:author]).to eql stub_user
describe 'if the project is not visible for the current_user' do
before do
projects = [stub_project]
Project.stub(:visible).and_return projects
projects.stub(:find_by_id).and_return(stub_project)
end
end.and_return(stub_issue)
end
it 'should return nil' do
controller.work_package.should be_nil
it 'should return a new issue on the project' do
controller.new_work_package.should == stub_issue
end
end
it 'should copy over attributes from another work_package provided as the source' do
controller.params[:copy_from] = 2
stub_issue.should_receive(:copy_from).with(2, :exclude => [:project_id])
describe 'when the sti_type is "Project"' do
it "should raise not allowed" do
controller.params = { :sti_type => 'Project',
:project_id => stub_project.id }.merge(params)
controller.new_work_package
expect { controller.work_package }.to raise_error ArgumentError
end
end
end
describe 'when the type is "Project"' do
it "should raise not allowed" do
controller.params = { :sti_type => 'Project' }
describe 'when providing neither id nor project_id (error)' do
it "should return nil" do
controller.params = {}
expect { controller.new_work_package }.to raise_error ArgumentError
controller.work_package.should be_nil
end
end
end
describe :project do
it "should be the work_packages's project" do
controller.stub!(:work_package).and_return(planning_element)
controller.stub(:work_package).and_return(planning_element)
controller.project.should == project
controller.project.should == planning_element.project
end
end
@ -747,6 +546,10 @@ describe WorkPackagesController do
end
describe :descendants do
before do
controller.stub(:work_package).and_return(planning_element)
end
it "should be empty" do
controller.descendants.should be_empty
end

@ -403,6 +403,22 @@ describe PermittedParams do
PermittedParams.new(params, user).new_work_package(:project => project).should == {}
end
it "should permit custom field values" do
hash = { "custom_field_values" => { "1" => "5" } }
params = ActionController::Parameters.new(:work_package => hash)
PermittedParams.new(params, user).new_work_package.should == hash
end
it "should remove custom field values that do not follow the schema 'id as string' => 'value as string'" do
hash = { "custom_field_values" => { "blubs" => "5", "5" => {"1" => "2"} } }
params = ActionController::Parameters.new(:work_package => hash)
PermittedParams.new(params, user).new_work_package.should == {}
end
end
describe :update_work_package do
@ -571,6 +587,22 @@ describe PermittedParams do
PermittedParams.new(params, user).update_work_package(:project => project).should == {}
end
it "should permit custom field values" do
hash = { "custom_field_values" => { "1" => "5" } }
params = ActionController::Parameters.new(:work_package => hash)
PermittedParams.new(params, user).new_work_package.should == hash
end
it "should remove custom field values that do not follow the schema 'id as string' => 'value as string'" do
hash = { "custom_field_values" => { "blubs" => "5", "5" => {"1" => "2"} } }
params = ActionController::Parameters.new(:work_package => hash)
PermittedParams.new(params, user).new_work_package.should == {}
end
end
describe :user do
@ -579,7 +611,6 @@ describe PermittedParams do
'mail',
'mail_notification',
'language',
'custom_field_values',
'custom_fields',
'identity_url',
'auth_source_id',
@ -609,5 +640,21 @@ describe PermittedParams do
PermittedParams.new(params, admin).user_update_as_admin.should == hash
end
it "should permit custom field values" do
hash = { "custom_field_values" => { "1" => "5" } }
params = ActionController::Parameters.new(:user => hash)
PermittedParams.new(params, admin).user_update_as_admin.should == hash
end
it "should remove custom field values that do not follow the schema 'id as string' => 'value as string'" do
hash = { "custom_field_values" => { "blubs" => "5", "5" => {"1" => "2"} } }
params = ActionController::Parameters.new(:user => hash)
PermittedParams.new(params, admin).user_update_as_admin.should == {}
end
end
end

@ -10,10 +10,14 @@
#++
require 'spec_helper'
require File.expand_path('../../support/shared/become_member', __FILE__)
describe Project do
include BecomeMember
let(:project) { FactoryGirl.build(:project) }
let(:admin) { FactoryGirl.create(:admin) }
let(:user) { FactoryGirl.create(:user) }
describe Project::STATUS_ACTIVE do
it "equals 1" do
@ -130,4 +134,26 @@ describe Project do
project.add_issue(attributes)
end
end
describe :find_visible do
it 'should find the project by id if the user is project member' do
become_member_with_permissions(project, user, :view_work_packages)
Project.find_visible(user, project.id).should == project
end
it 'should find the project by identifier if the user is project member' do
become_member_with_permissions(project, user, :view_work_packages)
Project.find_visible(user, project.identifier).should == project
end
it 'should not find the project by identifier if the user is no project member' do
expect { Project.find_visible(user, project.identifier) }.to raise_error ActiveRecord::RecordNotFound
end
it 'should not find the project by id if the user is no project member' do
expect { Project.find_visible(user, project.id) }.to raise_error ActiveRecord::RecordNotFound
end
end
end

@ -198,7 +198,7 @@ describe WorkPackage do
end
end
describe :update_with do
describe :update_by! do
#TODO remove once only WP exists
[:issue, :planning_element].each do |subclass|
@ -206,17 +206,17 @@ describe WorkPackage do
let(:instance) { send(subclass) }
it "should return true" do
instance.update_by(user, {}).should be_true
instance.update_by!(user, {}).should be_true
end
it "should set the values" do
instance.update_by(user, { :subject => "New subject" })
instance.update_by!(user, { :subject => "New subject" })
instance.subject.should == "New subject"
end
it "should create a journal with the journal's 'notes' attribute set to the supplied" do
instance.update_by(user, { :notes => "blubs" })
instance.update_by!(user, { :notes => "blubs" })
instance.journals.last.notes.should == "blubs"
end
@ -229,7 +229,7 @@ describe WorkPackage do
.with(instance, raw_attachments)
.and_return(attachment)
instance.update_by(user, { :attachments => raw_attachments })
instance.update_by!(user, { :attachments => raw_attachments })
end
it "should only attach the attachment when saving was successful" do
@ -238,13 +238,13 @@ describe WorkPackage do
Attachment.should_not_receive(:attach_files)
instance.update_by(user, { :subject => "", :attachments => raw_attachments })
instance.update_by!(user, { :subject => "", :attachments => raw_attachments })
end
it "should add a time entry" do
activity = FactoryGirl.create(:time_entry_activity)
instance.update_by(user, { :time_entry => { "hours" => "5",
instance.update_by!(user, { :time_entry => { "hours" => "5",
"activity_id" => activity.id.to_s,
"comments" => "blubs" } } )
@ -262,7 +262,7 @@ describe WorkPackage do
it "should not persist the time entry if the #{subclass}'s update fails" do
activity = FactoryGirl.create(:time_entry_activity)
instance.update_by(user, { :subject => '',
instance.update_by!(user, { :subject => '',
:time_entry => { "hours" => "5",
"activity_id" => activity.id.to_s,
"comments" => "blubs" } } )
@ -279,7 +279,7 @@ describe WorkPackage do
"activity_id" => "",
"comments" => "" }
instance.update_by(user, :time_entry => time_attributes)
instance.update_by!(user, :time_entry => time_attributes)
instance.should have(0).time_entries
end

@ -0,0 +1,21 @@
#-- copyright
# OpenProject is a project management system.
#
# Copyright (C) 2012-2013 the OpenProject Team
#
# This program is free software; you can redistribute it and/or
# modify it under the terms of the GNU General Public License version 3.
#
# See doc/COPYRIGHT.rdoc for more details.
#++
require File.expand_path('../../spec_helper', __FILE__)
require File.expand_path('../../support/permission_specs', __FILE__)
describe WorkPackagesController, "add_work_packages permission", :type => :controller do
include PermissionSpecs
check_permission_required_for('work_packages#new', :add_work_packages)
check_permission_required_for('work_packages#new_type', :add_work_packages)
check_permission_required_for('work_packages#create', :add_work_packages)
end

@ -0,0 +1,21 @@
#-- copyright
# OpenProject is a project management system.
#
# Copyright (C) 2012-2013 the OpenProject Team
#
# This program is free software; you can redistribute it and/or
# modify it under the terms of the GNU General Public License version 3.
#
# See doc/COPYRIGHT.rdoc for more details.
#++
require File.expand_path('../../spec_helper', __FILE__)
require File.expand_path('../../support/permission_specs', __FILE__)
describe WorkPackagesController, "edit_work_packages permission", :type => :controller do
include PermissionSpecs
check_permission_required_for('work_packages#edit', :edit_work_packages)
check_permission_required_for('work_packages#update', :edit_work_packages)
check_permission_required_for('work_packages#new_type', :edit_work_packages)
end

@ -0,0 +1,19 @@
#-- copyright
# OpenProject is a project management system.
#
# Copyright (C) 2012-2013 the OpenProject Team
#
# This program is free software; you can redistribute it and/or
# modify it under the terms of the GNU General Public License version 3.
#
# See doc/COPYRIGHT.rdoc for more details.
#++
require File.expand_path('../../spec_helper', __FILE__)
require File.expand_path('../../support/permission_specs', __FILE__)
describe WorkPackagesController, "view_work_packages permission", :type => :controller do
include PermissionSpecs
check_permission_required_for('work_packages#show', :view_work_packages)
end

@ -0,0 +1,50 @@
#-- encoding: UTF-8
#-- copyright
# OpenProject is a project management system.
#
# Copyright (C) 2012-2013 the OpenProject Team
#
# This program is free software; you can redistribute it and/or
# modify it under the terms of the GNU General Public License version 3.
#
# See doc/COPYRIGHT.rdoc for more details.
#++
require File.expand_path('../shared/become_member', __FILE__)
module PermissionSpecs
def self.included(base)
base.class_eval do
let(:project) { FactoryGirl.create(:project, :is_public => false) }
let(:current_user) { FactoryGirl.create(:user) }
include BecomeMember
def self.check_permission_required_for(controller_action, permission)
controller_name, action_name = controller_action.split('#')
it "should allow calling #{controller_action} when having the permission #{permission} permission" do
become_member_with_permissions(project, current_user, permission)
controller.send(:authorize, controller_name, action_name).should be_true
end
it "should prevent calling #{controller_action} when not having the permission #{permission} permission" do
become_member_with_permissions(project, current_user)
controller.send(:authorize, controller_name, action_name).should be_false
end
end
before do
# As failures generate a response we need to prevent calls to nil
controller.response = ActionController::TestResponse.new
User.stub(:current).and_return(current_user)
controller.instance_variable_set(:@project, project)
end
end
end
end

@ -0,0 +1,29 @@
#-- encoding: UTF-8
#-- copyright
# OpenProject is a project management system.
#
# Copyright (C) 2012-2013 the OpenProject Team
#
# This program is free software; you can redistribute it and/or
# modify it under the terms of the GNU General Public License version 3.
#
# See doc/COPYRIGHT.rdoc for more details.
#++
module BecomeMember
def self.included(base)
base.send(:include, InstanceMethods)
end
module InstanceMethods
def become_member_with_permissions(project, user, permissions = [])
permissions = Array(permissions)
role = FactoryGirl.create(:role, :permissions => permissions)
member = FactoryGirl.build(:member, :user => user, :project => project)
member.roles = [role]
member.save!
end
end
end
Loading…
Cancel
Save