leaves: switch association to user; update controllers/services/policy/inbox/specs (accepted changes sync)
This commit is contained in:
@@ -16,10 +16,10 @@ class Api::V1::Accounts::LeavesController < Api::V1::Accounts::BaseController
|
||||
end
|
||||
|
||||
def create
|
||||
account_user = find_or_authorize_account_user
|
||||
user = find_or_authorize_user
|
||||
service = Leaves::LeaveService.new(
|
||||
account: Current.account,
|
||||
account_user: account_user,
|
||||
user: user,
|
||||
current_user: Current.user
|
||||
)
|
||||
|
||||
@@ -86,12 +86,11 @@ class Api::V1::Accounts::LeavesController < Api::V1::Accounts::BaseController
|
||||
authorize @leave, :approve?
|
||||
end
|
||||
|
||||
def find_or_authorize_account_user
|
||||
def find_or_authorize_user
|
||||
if params[:user_id].present? && Current.account_user.administrator?
|
||||
user = Current.account.users.find(params[:user_id])
|
||||
Current.account.account_users.find_by!(user: user)
|
||||
Current.account.users.find(params[:user_id])
|
||||
else
|
||||
Current.account.account_users.find_by!(user: Current.user)
|
||||
Current.user
|
||||
end
|
||||
end
|
||||
|
||||
@@ -99,14 +98,14 @@ class Api::V1::Accounts::LeavesController < Api::V1::Accounts::BaseController
|
||||
@leave_service ||= if @leave
|
||||
Leaves::LeaveService.new(
|
||||
account: Current.account,
|
||||
account_user: @leave.account_user,
|
||||
user: @leave.user,
|
||||
current_user: Current.user
|
||||
)
|
||||
else
|
||||
# For index action, we don't have a specific leave
|
||||
Leaves::LeaveService.new(
|
||||
account: Current.account,
|
||||
account_user: nil,
|
||||
user: nil,
|
||||
current_user: Current.user
|
||||
)
|
||||
end
|
||||
|
||||
+7
-9
@@ -255,17 +255,15 @@ class Inbox < ApplicationRecord
|
||||
end
|
||||
|
||||
def filter_agents_on_leave(inbox_members_scope)
|
||||
# Get account users on active leave
|
||||
account_user_ids_on_leave = account.account_users
|
||||
.joins(:leaves)
|
||||
.where(leaves: { status: 'approved' })
|
||||
.where('leaves.start_date <= ? AND leaves.end_date >= ?', Date.current, Date.current)
|
||||
.pluck(:id)
|
||||
# Get users on active leave
|
||||
user_ids_on_leave = account.leaves
|
||||
.where(status: :approved)
|
||||
.where('start_date <= ? AND end_date >= ?', Date.current, Date.current)
|
||||
.pluck(:user_id)
|
||||
|
||||
return inbox_members_scope if account_user_ids_on_leave.empty?
|
||||
return inbox_members_scope if user_ids_on_leave.empty?
|
||||
|
||||
# Exclude inbox members whose account_users are on leave
|
||||
user_ids_on_leave = account.account_users.where(id: account_user_ids_on_leave).pluck(:user_id)
|
||||
# Exclude inbox members whose users are on leave
|
||||
inbox_members_scope.where.not(user_id: user_ids_on_leave)
|
||||
end
|
||||
|
||||
|
||||
+5
-7
@@ -31,11 +31,9 @@
|
||||
|
||||
class Leave < ApplicationRecord
|
||||
belongs_to :account
|
||||
belongs_to :account_user
|
||||
belongs_to :user
|
||||
belongs_to :approved_by, class_name: 'User', optional: true
|
||||
|
||||
has_one :user, through: :account_user
|
||||
|
||||
enum leave_type: {
|
||||
vacation: 0,
|
||||
sick: 1,
|
||||
@@ -92,10 +90,10 @@ class Leave < ApplicationRecord
|
||||
end
|
||||
|
||||
def no_overlapping_leaves
|
||||
overlapping_leaves = account_user.leaves
|
||||
.approved
|
||||
.where.not(id: id)
|
||||
.by_date_range(start_date, end_date)
|
||||
overlapping_leaves = user.leaves
|
||||
.approved
|
||||
.where.not(id: id)
|
||||
.by_date_range(start_date, end_date)
|
||||
|
||||
return unless overlapping_leaves.exists?
|
||||
|
||||
|
||||
@@ -7,7 +7,7 @@ class LeavePolicy < ApplicationPolicy
|
||||
|
||||
def show?
|
||||
# Users can view their own leaves or admins can view all
|
||||
record.account_user.user_id == user.id || @account_user.administrator?
|
||||
record.user_id == user.id || @account_user.administrator?
|
||||
end
|
||||
|
||||
def create?
|
||||
@@ -15,13 +15,13 @@ class LeavePolicy < ApplicationPolicy
|
||||
# When authorizing the class (not instance), allow any authenticated user
|
||||
return true if record.is_a?(Class)
|
||||
|
||||
record.account_user.user_id == user.id
|
||||
record.user_id == user.id
|
||||
end
|
||||
|
||||
def update?
|
||||
# Users can update their own pending/rejected leaves
|
||||
# Admins can update any leave
|
||||
@account_user.administrator? || (record.account_user.user_id == user.id && record.pending?)
|
||||
@account_user.administrator? || (record.user_id == user.id && record.pending?)
|
||||
end
|
||||
|
||||
def destroy?
|
||||
@@ -30,7 +30,7 @@ class LeavePolicy < ApplicationPolicy
|
||||
if @account_user.administrator?
|
||||
!record.approved?
|
||||
else
|
||||
record.account_user.user_id == user.id && record.pending?
|
||||
record.user_id == user.id && record.pending?
|
||||
end
|
||||
end
|
||||
|
||||
|
||||
@@ -100,6 +100,7 @@ class Leaves::LeaveApprovalService
|
||||
# If the leave starts today or is already active, reassign conversations
|
||||
return unless leave.start_date <= Date.current
|
||||
|
||||
ReassignConversationsJob.perform_later(leave.account_user)
|
||||
account_user = leave.account.account_users.find_by(user_id: leave.user_id)
|
||||
ReassignConversationsJob.perform_later(account_user)
|
||||
end
|
||||
end
|
||||
|
||||
@@ -1,10 +1,10 @@
|
||||
# frozen_string_literal: true
|
||||
|
||||
class Leaves::LeaveService
|
||||
pattr_initialize [:account!, :account_user!, :current_user!]
|
||||
pattr_initialize [:account!, :user!, :current_user!]
|
||||
|
||||
def create(params)
|
||||
leave = account_user.leaves.build(filtered_params(params))
|
||||
leave = user.leaves.build(filtered_params(params))
|
||||
leave.account = account
|
||||
|
||||
if leave.save
|
||||
@@ -44,9 +44,9 @@ class Leaves::LeaveService
|
||||
|
||||
scope = scope.by_date_range(filters[:start_date], filters[:end_date]) if filters[:start_date].present? && filters[:end_date].present?
|
||||
|
||||
scope = scope.joins(:account_user).where(account_users: { user_id: filters[:user_id] }) if filters[:user_id].present? && current_user_admin?
|
||||
scope = scope.where(user_id: filters[:user_id]) if filters[:user_id].present? && current_user_admin?
|
||||
|
||||
scope.includes(:account_user, :user, :approved_by).order(start_date: :desc)
|
||||
scope.includes(:user, :approved_by).order(start_date: :desc)
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
@@ -12,8 +12,8 @@ RSpec.describe 'Leaves API', type: :request do
|
||||
describe 'GET /api/v1/accounts/:account_id/leaves' do
|
||||
context 'when authenticated as an agent' do
|
||||
it 'returns only their own leaves' do
|
||||
leave1 = create(:leave, account_user: agent_account_user, account: account)
|
||||
create(:leave, account_user: account.account_users.find_by(user: another_agent), account: account)
|
||||
leave1 = create(:leave, user: agent, account: account)
|
||||
create(:leave, user: another_agent, account: account)
|
||||
|
||||
get "/api/v1/accounts/#{account.id}/leaves",
|
||||
headers: agent.create_new_auth_token,
|
||||
@@ -28,8 +28,8 @@ RSpec.describe 'Leaves API', type: :request do
|
||||
|
||||
context 'when authenticated as an admin' do
|
||||
it 'returns all leaves in the account' do
|
||||
create(:leave, account_user: agent_account_user, account: account)
|
||||
create(:leave, account_user: account.account_users.find_by(user: another_agent), account: account)
|
||||
create(:leave, user: agent, account: account)
|
||||
create(:leave, user: another_agent, account: account)
|
||||
|
||||
get "/api/v1/accounts/#{account.id}/leaves",
|
||||
headers: admin.create_new_auth_token,
|
||||
@@ -88,7 +88,7 @@ RSpec.describe 'Leaves API', type: :request do
|
||||
end
|
||||
|
||||
describe 'PUT /api/v1/accounts/:account_id/leaves/:id' do
|
||||
let(:leave) { create(:leave, account_user: agent_account_user, account: account) }
|
||||
let(:leave) { create(:leave, user: agent, account: account) }
|
||||
|
||||
context 'when authenticated as the leave owner' do
|
||||
it 'updates pending leave' do
|
||||
@@ -131,7 +131,7 @@ RSpec.describe 'Leaves API', type: :request do
|
||||
end
|
||||
|
||||
describe 'POST /api/v1/accounts/:account_id/leaves/:id/approve' do
|
||||
let(:leave) { create(:leave, account_user: agent_account_user, account: account) }
|
||||
let(:leave) { create(:leave, user: agent, account: account) }
|
||||
|
||||
context 'when authenticated as an admin' do
|
||||
it 'approves the leave' do
|
||||
@@ -161,7 +161,7 @@ RSpec.describe 'Leaves API', type: :request do
|
||||
end
|
||||
|
||||
describe 'POST /api/v1/accounts/:account_id/leaves/:id/reject' do
|
||||
let(:leave) { create(:leave, account_user: agent_account_user, account: account) }
|
||||
let(:leave) { create(:leave, user: agent, account: account) }
|
||||
|
||||
context 'when authenticated as an admin' do
|
||||
it 'rejects the leave with reason' do
|
||||
@@ -190,7 +190,7 @@ RSpec.describe 'Leaves API', type: :request do
|
||||
|
||||
describe 'DELETE /api/v1/accounts/:account_id/leaves/:id' do
|
||||
context 'when deleting own pending leave' do
|
||||
let(:leave) { create(:leave, account_user: agent_account_user, account: account) }
|
||||
let(:leave) { create(:leave, user: agent, account: account) }
|
||||
|
||||
it 'deletes the leave' do
|
||||
delete "/api/v1/accounts/#{account.id}/leaves/#{leave.id}",
|
||||
@@ -203,7 +203,7 @@ RSpec.describe 'Leaves API', type: :request do
|
||||
end
|
||||
|
||||
context 'when trying to delete approved leave' do
|
||||
let(:leave) { create(:leave, :approved, account_user: agent_account_user, account: account) }
|
||||
let(:leave) { create(:leave, :approved, user: agent, account: account) }
|
||||
|
||||
it 'returns unauthorized' do
|
||||
delete "/api/v1/accounts/#{account.id}/leaves/#{leave.id}",
|
||||
|
||||
@@ -3,7 +3,7 @@
|
||||
FactoryBot.define do
|
||||
factory :leave do
|
||||
account
|
||||
account_user
|
||||
user
|
||||
start_date { Date.current + 1.day }
|
||||
end_date { Date.current + 7.days }
|
||||
leave_type { 'vacation' }
|
||||
|
||||
@@ -34,7 +34,7 @@ RSpec.describe 'Inbox Leave Integration', skip: 'Enterprise feature', type: :mod
|
||||
|
||||
context 'when an agent is on approved leave' do
|
||||
before do
|
||||
create(:leave, :active, account_user: account_user1)
|
||||
create(:leave, :active, user: user1, account: account)
|
||||
end
|
||||
|
||||
it 'excludes the agent on leave' do
|
||||
@@ -46,8 +46,8 @@ RSpec.describe 'Inbox Leave Integration', skip: 'Enterprise feature', type: :mod
|
||||
|
||||
context 'when multiple agents are on leave' do
|
||||
before do
|
||||
create(:leave, :active, account_user: account_user1)
|
||||
create(:leave, :active, account_user: account_user2)
|
||||
create(:leave, :active, user: user1, account: account)
|
||||
create(:leave, :active, user: user2, account: account)
|
||||
end
|
||||
|
||||
it 'excludes all agents on leave' do
|
||||
@@ -58,7 +58,7 @@ RSpec.describe 'Inbox Leave Integration', skip: 'Enterprise feature', type: :mod
|
||||
|
||||
context 'when an agent has pending leave' do
|
||||
before do
|
||||
create(:leave, account_user: account_user1, status: 'pending')
|
||||
create(:leave, user: user1, account: account, status: 'pending')
|
||||
end
|
||||
|
||||
it 'does not exclude agents with pending leave' do
|
||||
@@ -69,7 +69,7 @@ RSpec.describe 'Inbox Leave Integration', skip: 'Enterprise feature', type: :mod
|
||||
|
||||
context 'when an agent has future approved leave' do
|
||||
before do
|
||||
create(:leave, :future, account_user: account_user1)
|
||||
create(:leave, :future, user: user1, account: account)
|
||||
end
|
||||
|
||||
it 'does not exclude agents with future leave' do
|
||||
@@ -80,7 +80,7 @@ RSpec.describe 'Inbox Leave Integration', skip: 'Enterprise feature', type: :mod
|
||||
|
||||
context 'when an agent has past leave' do
|
||||
before do
|
||||
create(:leave, :past, account_user: account_user1)
|
||||
create(:leave, :past, user: user1, account: account)
|
||||
end
|
||||
|
||||
it 'does not exclude agents with past leave' do
|
||||
@@ -91,7 +91,7 @@ RSpec.describe 'Inbox Leave Integration', skip: 'Enterprise feature', type: :mod
|
||||
|
||||
context 'with exclude_on_leave option' do
|
||||
before do
|
||||
create(:leave, :active, account_user: account_user1)
|
||||
create(:leave, :active, user: user1, account: account)
|
||||
end
|
||||
|
||||
it 'excludes agents on leave by default' do
|
||||
|
||||
@@ -5,9 +5,8 @@ require 'rails_helper'
|
||||
RSpec.describe Leave, type: :model do
|
||||
describe 'associations' do
|
||||
it { is_expected.to belong_to(:account) }
|
||||
it { is_expected.to belong_to(:account_user) }
|
||||
it { is_expected.to belong_to(:user) }
|
||||
it { is_expected.to belong_to(:approved_by).class_name('User').optional }
|
||||
it { is_expected.to have_one(:user).through(:account_user) }
|
||||
end
|
||||
|
||||
describe 'validations' do
|
||||
|
||||
Reference in New Issue
Block a user