Merge pull request #764 from justarrived/skill-proficiency

Skill proficiency

app/admin/skill.rb

@@ -1,4 +1,8 @@
 # frozen_string_literal: true
 ActiveAdmin.register Skill do
   menu parent: 'Settings'
+
+  permit_params do
+    [:name, :internal, :language_id]
+  end
 end

app/admin/user.rb

@@ -60,6 +60,37 @@
     redirect_to collection_path, notice: notice.join(' ')
   end
 
+  batch_action :add_and_remove_user_skill, form: lambda {
+    {
+      remove_skill: Skill.to_form_array(include_blank: true),
+      add_skill: Skill.to_form_array(include_blank: true)
+    }
+  } do |ids, inputs|
+    add_skill = inputs['add_skill']
+    remove_skill = inputs['remove_skill']
+
+    users = User.where(id: ids)
+    notice = []
+
+    unless add_skill.blank?
+      tag = Skill.find_by(id: add_skill)
+      users.each do |user|
+        UserSkill.safe_create(tag: tag, user: user)
+      end
+      notice << I18n.t('admin.user.batch_form.tag_added_notice', name: tag.name)
+    end
+
+    unless remove_skill.blank?
+      tag = Skill.find_by(id: remove_skill)
+      users.each do |user|
+        UserSkill.safe_destroy(tag: tag, user: user)
+      end
+      notice << I18n.t('admin.user.batch_form.tag_removed_notice', name: tag.name)
+    end
+
+    redirect_to collection_path, notice: notice.join(' ')
+  end
+
   batch_action :verify, confirm: I18n.t('admin.batch_action_confirm') do |ids|
     collection.where(id: ids).map { |u| u.update(verified: true) }
 
@@ -88,6 +119,7 @@
   filter :phone
   filter :ssn
   filter :tags
+  filter :skills
   filter :language
   filter :company
   filter :frilans_finans_id
@@ -99,6 +131,8 @@
   filter :managed
   filter :created_at
   # rubocop:disable Metrics/LineLength
+  filter :user_skills_proficiency_gteq, as: :select, collection: [nil, nil] + UserSkill::PROFICIENCY_RANGE.to_a
+  filter :user_skills_proficiency_by_admin_gteq, as: :select, collection: [nil, nil] + UserSkill::PROFICIENCY_RANGE.to_a
   filter :translations_description_cont, as: :string, label: I18n.t('admin.user.description')
   filter :translations_education_cont, as: :string, label: I18n.t('admin.user.education')
   filter :translations_competence_text_cont, as: :string, label: I18n.t('admin.user.competence_text')

app/admin/user_skill.rb

@@ -0,0 +1,33 @@
+# frozen_string_literal: true
+ActiveAdmin.register UserSkill do
+  menu parent: 'Users'
+
+  index do
+    selectable_column
+
+    column :id
+    column :name do |user_skills|
+      skill = user_skills.skill
+
+      link_to(
+        skill.name,
+        admin_users_path + AdminHelpers::Link.query(:user_skills_skill_id, skill.id)
+      )
+    end
+    column :proficiency
+    column :proficiency_by_admin
+    column :created_at
+
+    actions
+  end
+
+  permit_params do
+    [:user_id, :skill_id, :proficiency, :proficiency_by_admin]
+  end
+
+  controller do
+    def scoped_collection
+      super.includes(:skill)
+    end
+  end
+end

app/controllers/api/v1/skills_controller.rb

@@ -99,7 +99,7 @@ def destroy
       private
 
       def set_skill
-        @skill = Skill.find(params[:id])
+        @skill = policy_scope(Skill).find(params[:id])
       end
 
       def skill_params

app/controllers/api/v1/users/user_skills_controller.rb

@@ -54,6 +54,7 @@ def show
         param :data, Hash, desc: 'Top level key', required: true do
           param :attributes, Hash, desc: 'Skill attributes', required: true do
             param :id, Integer, desc: 'Skill id', required: true
+            param :proficiency, UserSkill::PROFICIENCY_RANGE.to_a, desc: 'Skill proficiency' # rubocop:disable Metrics/LineLength
           end
         end
         example Doxxer.read_example(UserSkill, method: :create)
@@ -64,6 +65,7 @@ def create
           authorize(@user_skill)
 
           @user_skill.skill = Skill.find_by(id: skill_params[:id])
+          @user_skill.proficiency = skill_params[:proficiency]
 
           if @user_skill.save
             api_render(@user_skill, status: :created)
@@ -79,7 +81,8 @@ def create
         def destroy
           authorize(@user_skill)
 
-          @user_skill.destroy
+          @user_skill.destroy unless @user_skill.touched_by_admin?
+
           head :no_content
         end
 
@@ -98,7 +101,7 @@ def set_user_skill
         end
 
         def skill_params
-          jsonapi_params.permit(:id)
+          jsonapi_params.permit(:id, :proficiency)
         end
 
         def pundit_user

app/models/skill.rb

@@ -10,6 +10,19 @@ class Skill < ApplicationRecord
 
   validates :name, uniqueness: true, length: { minimum: 3 }, allow_blank: false
   validates :language, presence: true
+
+  scope :visible, -> { where(internal: false) }
+
+  def self.to_form_array(include_blank: false)
+    form_array = pluck(:name, :id)
+    return form_array unless include_blank
+
+    [[I18n.t('admin.form.no_skill_chosen'), nil]] + form_array
+  end
+
+  def display_name
+    "##{id} #{name}"
+  end
 end
 
 # == Schema Information
@@ -21,6 +34,7 @@ class Skill < ApplicationRecord
 #  created_at  :datetime         not null
 #  updated_at  :datetime         not null
 #  language_id :integer
+#  internal    :boolean          default(FALSE)
 #
 # Indexes
 #

app/models/user_skill.rb

@@ -1,23 +1,41 @@
 # frozen_string_literal: true
 class UserSkill < ApplicationRecord
+  PROFICIENCY_RANGE = 1..10
+
   belongs_to :user
   belongs_to :skill
 
   validates :skill, presence: true
   validates :user, presence: true
   validates :skill, uniqueness: { scope: :user }
   validates :user, uniqueness: { scope: :skill }
+
+  scope :visible, -> { joins(:skill).where(skills: { internal: false }) }
+
+  def self.safe_create(skill:, user:)
+    find_or_create_by(user: user, skill: skill)
+  end
+
+  def self.safe_destroy(skill:, user:)
+    find_by(user: user, skill: skill)&.destroy!
+  end
+
+  def touched_by_admin?
+    !proficiency_by_admin.nil?
+  end
 end
 
 # == Schema Information
 #
 # Table name: user_skills
 #
-#  id         :integer          not null, primary key
-#  user_id    :integer
-#  skill_id   :integer
-#  created_at :datetime         not null
-#  updated_at :datetime         not null
+#  id                   :integer          not null, primary key
+#  user_id              :integer
+#  skill_id             :integer
+#  created_at           :datetime         not null
+#  updated_at           :datetime         not null
+#  proficiency          :integer
+#  proficiency_by_admin :integer
 #
 # Indexes
 #

app/policies/skill_policy.rb

@@ -1,5 +1,11 @@
 # frozen_string_literal: true
 class SkillPolicy < ApplicationPolicy
+  class Scope < Scope
+    def resolve
+      scope.visible
+    end
+  end
+
   def index?
     true
   end

app/policies/user_skill_policy.rb

@@ -1,5 +1,11 @@
 # frozen_string_literal: true
 class UserSkillPolicy < ApplicationPolicy
+  class Scope < Scope
+    def resolve
+      scope.visible
+    end
+  end
+
   Context = Struct.new(:user, :user_record)
 
   attr_reader :user_record

config/locales/admin/en.yml

@@ -9,6 +9,7 @@ en:
     time_from_now: '%{time} from now'
     form:
       no_tag_chosen: '-- No tag --'
+      no_tag_chosen: '-- No skill --'
     filter:
       near_address: "Near address (add 'km: XX' for distance)"
     recent_users:

db/migrate/20170102204144_add_proficiency_to_user_skills.rb

@@ -0,0 +1,7 @@
+# frozen_string_literal: true
+class AddProficiencyToUserSkills < ActiveRecord::Migration[5.0]
+  def change
+    add_column :user_skills, :proficiency, :integer
+    add_column :user_skills, :proficiency_by_admin, :integer
+  end
+end

db/migrate/20170102205042_add_internal_to_skills.rb

@@ -0,0 +1,6 @@
+# frozen_string_literal: true
+class AddInternalToSkills < ActiveRecord::Migration[5.0]
+  def change
+    add_column :skills, :internal, :boolean, default: false
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20170102123906) do
+ActiveRecord::Schema.define(version: 20170102205042) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -237,6 +237,7 @@
     t.datetime "updated_at",                null: false
     t.integer  "frilans_finans_invoice_id"
     t.index ["frilans_finans_invoice_id"], name: "index_invoices_on_frilans_finans_invoice_id", using: :btree
+    t.index ["job_user_id"], name: "index_invoices_on_job_user_id", using: :btree
     t.index ["job_user_id"], name: "index_invoices_on_job_user_id_uniq", unique: true, using: :btree
   end
 
@@ -395,9 +396,10 @@
 
   create_table "skills", force: :cascade do |t|
     t.string   "name"
-    t.datetime "created_at",  null: false
-    t.datetime "updated_at",  null: false
+    t.datetime "created_at",                  null: false
+    t.datetime "updated_at",                  null: false
     t.integer  "language_id"
+    t.boolean  "internal",    default: false
     t.index ["language_id"], name: "index_skills_on_language_id", using: :btree
     t.index ["name"], name: "index_skills_on_name", unique: true, using: :btree
   end
@@ -472,8 +474,10 @@
   create_table "user_skills", force: :cascade do |t|
     t.integer  "user_id"
     t.integer  "skill_id"
-    t.datetime "created_at", null: false
-    t.datetime "updated_at", null: false
+    t.datetime "created_at",           null: false
+    t.datetime "updated_at",           null: false
+    t.integer  "proficiency"
+    t.integer  "proficiency_by_admin"
     t.index ["skill_id", "user_id"], name: "index_user_skills_on_skill_id_and_user_id", unique: true, using: :btree
     t.index ["skill_id"], name: "index_user_skills_on_skill_id", using: :btree
     t.index ["user_id", "skill_id"], name: "index_user_skills_on_user_id_and_skill_id", unique: true, using: :btree

spec/controllers/users/user_skills_controller_spec.rb

@@ -20,7 +20,9 @@
 
   describe 'GET #index' do
     it 'assigns all user skills as @skills' do
-      user = FactoryGirl.create(:user_with_skills, skills_count: 1)
+      user = FactoryGirl.create(:user_with_skills, skills_count: 2)
+      user.user_skills.last.skill.update(internal: true)
+
       allow_any_instance_of(described_class).
         to(receive(:current_user).
         and_return(user))
@@ -30,6 +32,18 @@
       expect(assigns(:user_skills)).to eq([user_skill])
     end
 
+    it 'does not return internal skills' do
+      user = FactoryGirl.create(:user_with_skills, skills_count: 1)
+      user.user_skills.first.skill.update(internal: true)
+
+      allow_any_instance_of(described_class).
+        to(receive(:current_user).
+        and_return(user))
+
+      get :index, params: { user_id: user.to_param }
+      expect(assigns(:user_skills)).to eq([])
+    end
+
     it 'returns 200 ok status' do
       user = FactoryGirl.create(:user)
       allow_any_instance_of(described_class).
@@ -77,12 +91,13 @@
           params = {
             user_id: user.to_param,
             data: {
-              attributes: { id: skill.to_param }
+              attributes: { id: skill.to_param, proficiency: 7 }
             }
           }
           post :create, params: params, headers: valid_session
           expect(assigns(:user_skill)).to be_a(UserSkill)
           expect(assigns(:user_skill)).to be_persisted
+          expect(assigns(:user_skill).proficiency).to eq(7)
         end
 
         it 'returns created status' do
@@ -148,6 +163,16 @@
         end.to change(UserSkill, :count).by(-1)
       end
 
+      it 'does *not* destroy the requested user_skill if touched by admin' do
+        user_skill = user.user_skills.first
+        user_skill.update(proficiency_by_admin: 7)
+
+        expect do
+          params = { user_id: user.to_param, user_skill_id: user_skill.to_param }
+          delete :destroy, params: params, headers: valid_session
+        end.to change(UserSkill, :count).by(0)
+      end
+
       it 'returns no content status' do
         user_skill = user.user_skills.first
         params = { user_id: user.to_param, user_skill_id: user_skill.to_param }