Merge pull request #176 from justarrived/job-refactor

Job refactor

.ruby-style-guide.yml

@@ -17,6 +17,9 @@ Style/CollectionMethods:
     find: detect
     find_all: select
     reduce: inject
+Style/RedundantFreeze:
+  Description: "Checks usages of Object#freeze on immutable objects."
+  Enabled: false
 Style/DotPosition:
   Description: Checks the position of the dot in multi-line method calls.
   StyleGuide: https://github.com/bbatsov/ruby-style-guide#consistent-multi-line-chains

app/controllers/api/v1/jobs_controller.rb

@@ -50,7 +50,7 @@ def show
       def create
         authorize(Job)
 
-        @job = Job.new(job_owner_params)
+        @job = Job.new(permitted_attributes)
         @job.owner_user_id = current_user.id
 
         if @job.save
@@ -87,23 +87,17 @@ def create
       def update
         authorize(@job)
 
-        notify_klass = nil
-        should_notify = false
-        if @job.owner == current_user
-          @job.assign_attributes(job_owner_params)
+        @job.assign_attributes(permitted_attributes)
+
+        notify_klass = NilNotifier
+        if job_policy.owner? && @job.send_performed_accept_notice?
           notify_klass = JobPerformedAcceptNotifier
-          should_notify = @job.send_performed_accept_notice?
-        elsif @job.accepted_applicant?(current_user)
-          @job.assign_attributes(job_user_params)
+        elsif job_policy.accepted_applicant? && @job.send_performed_notice?
           notify_klass = JobPerformedNotifier
-          should_notify = @job.send_performed_notice?
-        else
-          render json: { error: I18n.t('invalid_credentials') }, status: :unauthorized
-          return
         end
 
         if @job.save
-          notify_klass.call(job: @job) if should_notify
+          notify_klass.call(job: @job)
           render json: @job, status: :ok
         else
           render json: @job.errors, status: :unprocessable_entity
@@ -125,16 +119,12 @@ def set_job
         @job = Job.find(params[:job_id])
       end
 
-      def job_owner_params
-        owner_params = [
-          :max_rate, :performed_accept, :description, :job_date, :street, :zip,
-          :name, :hours, :language_id, skill_ids: []
-        ]
-        jsonapi_params.permit(*owner_params)
+      def job_policy
+        policy(@job || Job.new)
       end
 
-      def job_user_params
-        jsonapi_params.permit(:performed)
+      def permitted_attributes
+        jsonapi_params.permit(job_policy.permitted_attributes)
       end
     end
   end

app/models/job.rb

@@ -37,6 +37,10 @@ def self.matches_user(user, distance: 20, strict_match: false)
       order_by_matching_skills(user, strict_match: strict_match)
   end
 
+  def owner?(user)
+    !owner.nil? && owner == user
+  end
+
   # NOTE: You need to call this __before__ the record is saved/updated
   #       otherwise it will always return false
   def send_performed_accept_notice?
@@ -50,7 +54,7 @@ def send_performed_notice?
   end
 
   def accepted_applicant?(user)
-    accepted_applicant == user
+    !accepted_applicant.nil? && accepted_applicant == user
   end
 
   def accepted_applicant

app/notifiers/nil_notifier.rb

@@ -0,0 +1,4 @@
+class NilNotifier
+  def self.call(**_args)
+  end
+end

app/policies/application_policy.rb

@@ -52,4 +52,8 @@ def admin?
   def user?
     !user.nil?
   end
+
+  def no_user?
+    !user?
+  end
 end

app/policies/chat_policy.rb

@@ -12,14 +12,4 @@ def resolve
   def index?
     admin?
   end
-
-  private
-
-  def admin?
-    user? && user.admin?
-  end
-
-  def user?
-    !user.nil?
-  end
 end

app/policies/job_policy.rb

@@ -9,15 +9,50 @@ def create?
     user?
   end
 
-  alias_method :update?, :create?
+  def update?
+    admin? || owner? || accepted_applicant?
+  end
 
   def matching_users?
-    user? && user.admin? || user == record.owner
+    admin? || owner?
+  end
+
+  def permitted_attributes
+    if admin?
+      admin_params
+    elsif !record.persisted? || owner?
+      owner_params
+    elsif accepted_applicant?
+      accepted_applicant_params
+    else
+      []
+    end
+  end
+
+  # Methods that don't match any controller action
+
+  def owner?
+    record.owner?(user)
+  end
+
+  def accepted_applicant?
+    record.accepted_applicant?(user)
   end
 
   private
 
-  def user?
-    !user.nil?
+  def admin_params
+    owner_params + accepted_applicant_params
+  end
+
+  def owner_params
+    [
+      :max_rate, :performed_accept, :description, :job_date, :street, :zip,
+      :name, :hours, :language_id, skill_ids: []
+    ]
+  end
+
+  def accepted_applicant_params
+    [:performed]
   end
 end

app/policies/language_policy.rb

@@ -11,10 +11,4 @@ def create?
 
   alias_method :update?, :create?
   alias_method :destroy?, :create?
-
-  private
-
-  def admin?
-    !user.nil? && user.admin?
-  end
 end

app/policies/skill_policy.rb

@@ -11,14 +11,4 @@ def create?
 
   alias_method :update?, :create?
   alias_method :destroy?, :create?
-
-  private
-
-  def admin?
-    user? && user.admin?
-  end
-
-  def user?
-    !user.nil?
-  end
 end

app/policies/user_language_policy.rb

@@ -24,14 +24,6 @@ def create?
 
   private
 
-  def admin?
-    user? && user.admin?
-  end
-
-  def user?
-    !user.nil?
-  end
-
   def admin_or_self?
     admin? || user == record.user
   end

app/policies/user_policy.rb

@@ -18,19 +18,7 @@ def show?
 
   private
 
-  def no_user?
-    !user?
-  end
-
-  def user?
-    !user.nil?
-  end
-
   def admin_or_self?
     admin? || user == record
   end
-
-  def admin?
-    user? && user.admin?
-  end
 end

app/policies/user_skill_policy.rb

@@ -24,14 +24,6 @@ def create?
 
   private
 
-  def admin?
-    user? && user.admin?
-  end
-
-  def user?
-    !user.nil?
-  end
-
   def admin_or_self?
     admin? || user == record.user
   end

spec/models/job_spec.rb

@@ -24,6 +24,29 @@
     end
   end
 
+  describe '#owner?' do
+    let(:user) { FactoryGirl.build(:user) }
+    let(:job) { FactoryGirl.build(:job, owner: user) }
+
+    it 'returns true if user is owner' do
+      expect(job.owner?(user)).to eq(true)
+    end
+
+    it 'returns true if user is owner' do
+      a_user = FactoryGirl.build(:user)
+      expect(job.owner?(a_user)).to eq(false)
+    end
+
+    it 'returns false if owner is nil and user is not nil' do
+      a_user = FactoryGirl.build(:user)
+      expect(job.owner?(a_user)).to eq(false)
+    end
+
+    it 'returns false if owner is nil and user is nil' do
+      expect(job.owner?(nil)).to eq(false)
+    end
+  end
+
   describe '#send_performed_accept_notice?' do
     it 'returns true if notice should be sent' do
       job = described_class.new
@@ -91,6 +114,17 @@
 
       expect(job.accepted_applicant?(applicant)).to eq(true)
     end
+
+    it 'returns false if owner is nil and user is not' do
+      nil_job = Job.new
+      a_user = FactoryGirl.build(:user)
+      expect(nil_job.accepted_applicant?(a_user)).to eq(false)
+    end
+
+    it 'returns false if owner is nil and user is nil' do
+      nil_job = Job.new
+      expect(nil_job.accepted_applicant?(nil)).to eq(false)
+    end
   end
 end
 

spec/notifiers/nil_notifier_spec.rb

@@ -0,0 +1,13 @@
+require 'spec_helper'
+
+RSpec.describe NilNotifier do
+  subject { NilNotifier }
+
+  it 'accepts any keyword arguments' do
+    expect(subject.call(wat: nil, man: nil)).to eq(nil)
+  end
+
+  it 'does not accept regular arguments' do
+    expect { subject.call(nil) }.to raise_error(ArgumentError)
+  end
+end