Shipments, refactoring of HasState and strong params

 * IMPORTANT: tests for the new functionality are still missing!!
 * New type of state machine: shipment (for booth boxes and similar)
 * New roles to support the shipment workflow
 * Strong parameters instead of protected_attributes
 * Refactoring of HasState, moving more and more logic into the concern
 * State machines (Request, Reimbursement, Shipment) can only be edited by the
   requester and only in initial state. Added additional actions to support
   setting the approved amount and similar actions.

Gemfile

@@ -52,8 +52,6 @@ gem "state_machine"
 # Added to avoid an error in every rake execution caused by lib/tasks/doc.rake
 # (at least until we figure out a cleaner solution)
 gem "yard"
-# For using rails3-style attributes protection
-gem 'protected_attributes'
 # delayed_job must appear after protected_attributes
 gem 'delayed_job_active_record'
 

Gemfile.lock

@@ -162,8 +162,6 @@ GEM
     prawn_rails (0.0.11)
       prawn (>= 0.11.1)
       railties (>= 3.0.0)
-    protected_attributes (1.0.5)
-      activemodel (>= 4.0.1, < 5.0)
     rack (1.5.2)
     rack-test (0.6.2)
       rack (>= 1.0)
@@ -303,7 +301,6 @@ DEPENDENCIES
   localized_country_select
   pdf-reader
   prawn_rails
-  protected_attributes
   rails (= 4.0.2)
   ransack
   redcarpet

app/controllers/application_controller.rb

@@ -17,13 +17,15 @@ class ApplicationController < ActionController::Base
   protected
 
   def prepare_for_nested_resource
-    @request ||= Request.find(params[:request_id])
-    if request.fullpath.include?("/reimbursement/")
-      @parent = @reimbursement = @request.reimbursement
+    machine = params[:machine].to_s
+    klass = machine.camelize.constantize
+    if machine == 'reimbursement'
+      @request = Request.find(params[:request_id])
+      @parent = @request.reimbursement
       @back_path = request_reimbursement_path(@request)
     else
-      @parent = @request
-      @back_path = request_path(@request)
+      @parent ||= klass.find(params["#{machine}_id"])
+      @back_path = url_for(@parent)
     end
   end
 

app/controllers/budgets_controller.rb

@@ -9,4 +9,8 @@ def collection
     @q.sorts = "name asc" if @q.sorts.empty?
     @budgets ||= @q.result(:distinct => true).page(params[:page]).per(20)
   end
+
+  def permitted_params
+    params.permit(:budget => [:name, :description, :amount, :currency, :event_ids => []])
+  end
 end

app/controllers/comments_controller.rb

@@ -24,9 +24,11 @@ def create
 
   def load_command_and_authorize
     prepare_for_nested_resource
-    @comment = @parent.comments.build(params[:comment])
-    if action_name.to_sym == :new && Comment.private_role?(current_user.profile.role_name)
-      @comment.private = true
+    if action_name.to_sym == :new
+      @comment = @parent.comments.build(:private => Comment.private_role?(current_user.profile.role_name))
+    else
+      @comment = @parent.comments.build(:body => params[:comment][:body],
+                                        :private => params[:comment][:private])
     end
     authorize! :create, @comment
   end

app/controllers/events_controller.rb

@@ -2,7 +2,7 @@ class EventsController < InheritedResources::Base
   respond_to :html, :js, :json
   skip_before_filter :authenticate_and_audit_user, :only => [:index, :show]
   skip_load_and_authorize_resource :only => [:index, :show]
-  before_filter :protect_attributes, :only => [:create, :update]
+  before_filter :set_types
 
   protected
 
@@ -16,12 +16,20 @@ def collection
     @events ||= @q.result(:distinct => true).page(params[:page]).per(20)
   end
 
-  def protect_attributes
-    if cannot? :validate, resource
+  def set_types
+    @shipment_types = TravelSupport::Config.setting('shipment_types')
+  end
+
+  def permitted_params
+    attrs = [:name, :description, :start_date, :end_date, :url, :country_code,
+             :validated, :visa_letters, :request_creation_deadline,
+             :reimbursement_creation_deadline, :budget_id, :shipment_type ]
+    if cannot? :validate, Event.new
       Event.validation_attributes.each do |att|
-        params[:event].delete(att)
+        attrs.delete(att)
       end
     end
-    params[:event].delete(:budget_id) if cannot? :read, Budget
+    attrs.delete(:budget_id) if cannot? :read, Budget
+    params.permit(:event => attrs)
   end
 end

app/controllers/expenses_approvals_controller.rb

@@ -0,0 +1,27 @@
+class ExpensesApprovalsController < ApplicationController
+  respond_to :html, :json
+  skip_load_and_authorize_resource
+  before_filter :load_request_and_authorize
+
+  def update
+    if p = params[:expenses_approval]
+      @request.expenses.each do |expense|
+        expense.approved_amount = p[:"amount_#{expense.id}"]
+        expense.approved_currency = p[:"currency_#{expense.id}"]
+        expense.save
+      end
+    end
+    respond_to do |format|
+      format.html { redirect_to @request,
+                        :notice => I18n.t(:"flash.actions.update.notice", :resource_name => 'request')  }
+      format.json { render :json => {:request => @request} }
+    end
+  end
+
+  protected
+
+  def load_request_and_authorize
+    @request = Request.includes(:expenses).find(params[:request_id])
+    authorize! :approve, @request
+  end
+end

app/controllers/payments_controller.rb

@@ -39,4 +39,9 @@ def set_breadcrumbs
                       :url => request_reimbursement_path(parent.request)}
     @breadcrumbs << {:label => Payment.model_name.human(:count => 2) }
   end
+
+  def permitted_params
+    params.permit(:payment => [:date, :amount, :currency, :cost_amount, :cost_currency,
+                               :method, :code, :subject, :notes, :file, :file_cache])
+  end
 end

app/controllers/registrations_controller.rb

@@ -7,7 +7,8 @@ def update
     end
 
     @user = User.find(current_user.id)
-    if @user.update_attributes(params[:user])
+    attrs = params.require(:user).permit(:nickname, :email, :locale)
+    if @user.update_attributes(attrs)
       # Sign in the user bypassing validation in case his password changed
       #set_flash_key :notice, :updated
       flash[:notice] = 'Dreamland'

app/controllers/reimbursements_controller.rb

@@ -50,4 +50,15 @@ def set_breadcrumbs
       @breadcrumbs << {:label => Reimbursement.model_name.human, :url => resource_path }
     end
   end
+
+  def permitted_params
+    params.permit(:reimbursement => [  :description,
+                                      {:request_attributes => [{:expenses_attributes => [:id, :total_amount,
+                                                                                         :authorized_amount]}]},
+                                      {:attachments_attributes => [:id, :title, :file, :file_cache, :_destroy]},
+                                      {:links_attributes => [:id, :title, :url, :_destroy]},
+                                      {:bank_account_attributes => [:holder, :bank_name, :iban, :bic, :national_bank_code,
+                                                                    :format, :national_account_code, :country_code,
+                                                                    :bank_postal_address]}])
+  end
 end

app/controllers/requests_controller.rb

@@ -47,4 +47,10 @@ def load_subjects
     @subjects = TravelSupport::Config.setting :request_expense_subjects
   end
 
+  def permitted_params
+    params.permit(:request => [ :event_id, :description, :visa_letter,
+                                {:expenses_attributes => [:id, :subject, :description,
+                                                          :estimated_amount, :estimated_currency]}])
+  end
+
 end

app/controllers/shipments_controller.rb

@@ -0,0 +1,42 @@
+class ShipmentsController < InheritedResources::Base
+  respond_to :html, :js, :json
+  skip_load_resource :only => [:index, :new]
+  before_filter :set_states_collection, :only => [:index]
+
+  def show
+    # We don't want to break the normal process if something goes wrong
+    resource.user.profile.refresh rescue nil
+    show!
+  end
+
+  def create
+    @shipment ||= Shipment.new(params[:shipment])
+    @shipment.user = current_user
+    create!
+  end
+
+  def new
+    @shipment ||= Shipment.new(params[:shipment])
+    @shipment.event = Event.find(params[:event_id])
+    @shipment.user = current_user
+    authorize! :create, @shipment
+    new!
+  end
+
+  protected
+
+  def collection
+    @q ||= end_of_association_chain.accessible_by(current_ability).search(params[:q])
+    @q.sorts = "id asc" if @q.sorts.empty?
+    @all_shipments ||= @q.result(:distinct => true)
+    @shipments ||= @all_shipments.page(params[:page]).per(20)
+  end
+
+  def set_states_collection
+    @states_collection = Shipment.state_machines[:state].states.map {|s| [ s.human_name, s.value] }
+  end
+
+  def permitted_params
+    params.permit(:shipment => [:event_id, :description, :delivery_address])
+  end
+end

app/controllers/state_adjustments_controller.rb

@@ -18,7 +18,8 @@ def create
   def load_adjustment_and_authorize
     prepare_for_nested_resource
     authorize! :adjust_state, @parent
-    @state_adjustment = @parent.state_adjustments.build(params[:state_adjustment])
+    attrs = params[:state_adjustment] ? params[:state_adjustment].permit(:notes, :to) : {}
+    @state_adjustment = @parent.state_adjustments.build(attrs)
     @state_names = @parent.class.state_machines[:state].states.map(&:name)
   end
 end

app/controllers/state_transitions_controller.rb

@@ -27,7 +27,8 @@ def create
 
   def load_transition_and_authorize
     prepare_for_nested_resource
-    @state_transition = @parent.transitions.build(params[:state_transition])
+    attrs = params.require(:state_transition).permit(:notes, :state_event)
+    @state_transition = @parent.transitions.build(attrs)
     authorize! @state_transition.state_event.to_sym, @parent
   end
 end

app/controllers/user_profiles_controller.rb

@@ -1,10 +1,13 @@
 class UserProfilesController < ApplicationController
   force_ssl :unless => Proc.new { Rails.env.test? || Rails.env.development? }
   before_filter :set_user_and_profile
-  before_filter :remove_role_from_params, :only => [:update, :update_password]
 
   def update
-    if @profile.update_attributes(params[:user])
+    attrs = params.require(:user).permit(:country_code, :full_name, :location,
+                :passport, :alternate_id_document, :birthday, :phone_number,
+                :second_phone_number, :website, :blog, :description,
+                :postal_address, :zip_code)
+    if @profile.update_attributes(attrs)
       flash[:notice] = I18n.t(:profile_updated)
       redirect_to profile_path
     else
@@ -14,7 +17,7 @@ def update
   end
 
   def update_password
-    if @user.update_attributes(params[:user])
+    if @user.update_attributes(params.require(:user).permit(:password, :password_confirmation))
       # Sign in the user by passing validation in case his password changed
       sign_in @user, :bypass => true
       flash[:notice] = I18n.t(:password_updated)
@@ -30,13 +33,6 @@ def set_user_and_profile
     @profile.refresh
   end
 
-  # To prevent users changing their own role
-  def remove_role_from_params
-    params[:user].delete("role")
-    params[:user].delete("role_id")
-    params[:user].delete("role_name")
-  end
-
   def users_controller?
     true
   end

app/models/ability.rb

@@ -28,6 +28,7 @@ def initialize(user)
     #
     # See the wiki for details: https://github.com/ryanb/cancan/wiki/Defining-Abilities
 
+    can :manage, Shipment
     can :manage, User, :id => user.id
     can :manage, UserProfile, :user_id => user.id
     can [:read, :create], Event
@@ -51,11 +52,11 @@ def initialize(user)
         r.event && r.event.accepting_requests?
       end
       can :create, RequestExpense do |e|
-        e.request && e.request.editable_by_requester? && e.request.user == user
+        e.request && e.request.editable? && e.request.user == user
       end
       can :read, Request, :user_id => user.id
       can :update, Request do |r|
-        r.user == user && r.editable_by_requester?
+        r.user == user && r.editable?
       end
       can :destroy, Request do |r|
         r.user == user && r.can_be_destroyed?
@@ -74,7 +75,7 @@ def initialize(user)
       end
       can :read, Reimbursement, :user_id => user.id
       can :update, Reimbursement do |r|
-        r.user == user && r.editable_by_requester?
+        r.user == user && r.editable?
       end
       [:submit, :roll_back, :cancel].each do |action|
         can action, Reimbursement do |r|
@@ -87,15 +88,15 @@ def initialize(user)
         a.reimbursement.user == user
       end
       can [:create, :update, :destroy], ReimbursementAttachment do |a|
-        a.reimbursement.user == user && a.reimbursement.editable_by_requester?
+        a.reimbursement.user == user && a.reimbursement.editable?
       end
 
       # Reimbursement's bank account
       can :read, BankAccount do |a|
         a.reimbursement.user == user
       end
       can [:create, :update], BankAccount do |a|
-        a.reimbursement.user == user && a.reimbursement.editable_by_requester?
+        a.reimbursement.user == user && a.reimbursement.editable?
       end
 
       # Reimbursement's payments
@@ -140,9 +141,6 @@ def initialize(user)
 
       # Requests
       can :read, Request
-      can :update, Request do |r|
-        r.editable_by_tsp?
-      end
       can :cancel, Request do |r|
         r.cancelable_by_tsp?
       end
@@ -156,9 +154,6 @@ def initialize(user)
 
       # Reimbursements
       can :read, Reimbursement
-      can :update, Reimbursement do |r|
-        r.editable_by_tsp?
-      end
       can :cancel, Reimbursement do |r|
         r.cancelable_by_tsp?
       end

app/models/bank_account.rb

@@ -3,8 +3,6 @@
 # Defined as a separate model in the shake of cleanest.
 #
 class BankAccount < ActiveRecord::Base
-  attr_accessible :holder, :bank_name, :iban, :bic, :national_bank_code, :format,
-          :national_account_code, :country_code, :bank_postal_address
 
   # The associated reimbursement
   belongs_to :reimbursement, :inverse_of => :bank_account

app/models/budget.rb

@@ -8,7 +8,6 @@
 # with the same currency, the amounts are added.
 #
 class Budget < ActiveRecord::Base
-  attr_accessible :name, :description, :amount, :currency, :event_ids
   # Events that are covered by the budget
   has_many :events
 

app/models/comment.rb

@@ -5,7 +5,6 @@
 # with the requester-.
 #
 class Comment < ActiveRecord::Base
-  attr_accessible :body, :private
   # The associated state machine (request, reimbursement...)
   belongs_to :machine, :polymorphic => true, :inverse_of => :comments
   # The author of the note