diff --git a/config/routes.rb b/config/routes.rb index f32094ad8..a679782f4 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -96,7 +96,7 @@ Rails.application.routes.draw do post :execute, on: :member end resources :sla_policies, only: [:index, :create, :show, :update, :destroy] - resources :leaves, only: [:index, :create, :show, :update, :destroy] do + resources :leave_records, only: [:index, :create, :show, :update, :destroy] do member do patch :approve patch :reject diff --git a/enterprise/app/controllers/api/v1/accounts/leave_records_controller.rb b/enterprise/app/controllers/api/v1/accounts/leave_records_controller.rb new file mode 100644 index 000000000..9e9d621cc --- /dev/null +++ b/enterprise/app/controllers/api/v1/accounts/leave_records_controller.rb @@ -0,0 +1,48 @@ +class Api::V1::Accounts::LeaveRecordsController < Api::V1::Accounts::EnterpriseAccountsController + before_action :fetch_leave_record, only: [:show, :update, :destroy, :approve, :reject] + before_action :check_authorization, only: [:index, :create] + before_action :check_record_authorization, only: [:show, :update, :destroy, :approve, :reject] + + def index + @leave_records = policy_scope(Current.account.leave_records) + end + + def show; end + + def create + @leave_record = Current.account.leave_records.create!(permitted_params.merge(user: current_user)) + end + + def update + @leave_record.update!(permitted_params) + end + + def destroy + @leave_record.destroy! + head :ok + end + + def approve + @leave_record.approve!(current_user) + render :show + end + + def reject + @leave_record.reject!(current_user) + render :show + end + + private + + def permitted_params + params.require(:leave_record).permit(:start_date, :end_date, :leave_type, :reason) + end + + def fetch_leave_record + @leave_record = policy_scope(Current.account.leave_records).find(params[:id]) + end + + def check_record_authorization + authorize(@leave_record) + end +end diff --git a/enterprise/app/controllers/api/v1/accounts/leaves_controller.rb b/enterprise/app/controllers/api/v1/accounts/leaves_controller.rb deleted file mode 100644 index 596943d0b..000000000 --- a/enterprise/app/controllers/api/v1/accounts/leaves_controller.rb +++ /dev/null @@ -1,43 +0,0 @@ -class Api::V1::Accounts::LeavesController < Api::V1::Accounts::EnterpriseAccountsController - before_action :fetch_leave, only: [:show, :update, :destroy, :approve, :reject] - before_action :check_authorization - - def index - @leaves = policy_scope(Current.account.leaves) - end - - def show; end - - def create - @leave = Current.account.leaves.create!(permitted_params.merge(user: current_user)) - end - - def update - @leave.update!(permitted_params) - end - - def destroy - @leave.destroy! - head :ok - end - - def approve - @leave.approve!(current_user) - render :show - end - - def reject - @leave.reject!(current_user) - render :show - end - - private - - def permitted_params - params.require(:leave).permit(:start_date, :end_date, :leave_type, :reason) - end - - def fetch_leave - @leave = Current.account.leaves.find_by(id: params[:id]) - end -end diff --git a/enterprise/app/models/enterprise/concerns/account.rb b/enterprise/app/models/enterprise/concerns/account.rb index d0aba4f23..e29803645 100644 --- a/enterprise/app/models/enterprise/concerns/account.rb +++ b/enterprise/app/models/enterprise/concerns/account.rb @@ -5,7 +5,7 @@ module Enterprise::Concerns::Account has_many :sla_policies, dependent: :destroy_async has_many :applied_slas, dependent: :destroy_async has_many :custom_roles, dependent: :destroy_async - has_many :leaves, dependent: :destroy_async, class_name: 'Leave' + has_many :leave_records, dependent: :destroy_async, class_name: 'LeaveRecord' has_many :captain_assistants, dependent: :destroy_async, class_name: 'Captain::Assistant' has_many :captain_assistant_responses, dependent: :destroy_async, class_name: 'Captain::AssistantResponse' diff --git a/enterprise/app/models/enterprise/concerns/user.rb b/enterprise/app/models/enterprise/concerns/user.rb index 03347848d..4c8d3761c 100644 --- a/enterprise/app/models/enterprise/concerns/user.rb +++ b/enterprise/app/models/enterprise/concerns/user.rb @@ -6,8 +6,8 @@ module Enterprise::Concerns::User has_many :captain_responses, class_name: 'Captain::AssistantResponse', dependent: :nullify, as: :documentable has_many :copilot_threads, dependent: :destroy_async - has_many :leaves, dependent: :destroy_async, class_name: 'Leave' - has_many :approved_leaves, class_name: 'Leave', foreign_key: 'approved_by_id', dependent: :nullify, inverse_of: :approver + has_many :leave_records, dependent: :destroy_async, class_name: 'LeaveRecord' + has_many :approved_leave_records, class_name: 'LeaveRecord', foreign_key: 'approved_by_id', dependent: :nullify, inverse_of: :approver end def ensure_installation_pricing_plan_quantity diff --git a/enterprise/app/models/leave.rb b/enterprise/app/models/leave_record.rb similarity index 94% rename from enterprise/app/models/leave.rb rename to enterprise/app/models/leave_record.rb index 89deb5202..f2d95c2f6 100644 --- a/enterprise/app/models/leave.rb +++ b/enterprise/app/models/leave_record.rb @@ -22,10 +22,12 @@ # index_leaves_on_approved_by_id (approved_by_id) # index_leaves_on_user_id (user_id) # -class Leave < ApplicationRecord +class LeaveRecord < ApplicationRecord + self.table_name = 'leaves' + belongs_to :account belongs_to :user - belongs_to :approver, class_name: 'User', foreign_key: 'approved_by_id', optional: true, inverse_of: :approved_leaves + belongs_to :approver, class_name: 'User', foreign_key: 'approved_by_id', optional: true, inverse_of: :approved_leave_records enum leave_type: { annual: 0, @@ -77,7 +79,7 @@ class Leave < ApplicationRecord end def overlaps_with?(other_leave) - return false unless other_leave.is_a?(Leave) + return false unless other_leave.is_a?(LeaveRecord) start_date <= other_leave.end_date && end_date >= other_leave.start_date end diff --git a/enterprise/app/policies/leave_policy.rb b/enterprise/app/policies/leave_record_policy.rb similarity index 96% rename from enterprise/app/policies/leave_policy.rb rename to enterprise/app/policies/leave_record_policy.rb index 4348e46d6..f1f4b7375 100644 --- a/enterprise/app/policies/leave_policy.rb +++ b/enterprise/app/policies/leave_record_policy.rb @@ -1,4 +1,4 @@ -class LeavePolicy < ApplicationPolicy +class LeaveRecordPolicy < ApplicationPolicy def index? @account_user.administrator? || @account_user.agent? end diff --git a/enterprise/app/views/api/v1/accounts/leave_records/create.json.jbuilder b/enterprise/app/views/api/v1/accounts/leave_records/create.json.jbuilder new file mode 100644 index 000000000..2366f65b0 --- /dev/null +++ b/enterprise/app/views/api/v1/accounts/leave_records/create.json.jbuilder @@ -0,0 +1 @@ +json.partial! 'api/v1/models/leave_record', formats: [:json], leave_record: @leave_record diff --git a/enterprise/app/views/api/v1/accounts/leave_records/index.json.jbuilder b/enterprise/app/views/api/v1/accounts/leave_records/index.json.jbuilder new file mode 100644 index 000000000..5e9099182 --- /dev/null +++ b/enterprise/app/views/api/v1/accounts/leave_records/index.json.jbuilder @@ -0,0 +1,3 @@ +json.array! @leave_records do |leave_record| + json.partial! 'api/v1/models/leave_record', formats: [:json], leave_record: leave_record +end diff --git a/enterprise/app/views/api/v1/accounts/leave_records/show.json.jbuilder b/enterprise/app/views/api/v1/accounts/leave_records/show.json.jbuilder new file mode 100644 index 000000000..2366f65b0 --- /dev/null +++ b/enterprise/app/views/api/v1/accounts/leave_records/show.json.jbuilder @@ -0,0 +1 @@ +json.partial! 'api/v1/models/leave_record', formats: [:json], leave_record: @leave_record diff --git a/enterprise/app/views/api/v1/accounts/leave_records/update.json.jbuilder b/enterprise/app/views/api/v1/accounts/leave_records/update.json.jbuilder new file mode 100644 index 000000000..2366f65b0 --- /dev/null +++ b/enterprise/app/views/api/v1/accounts/leave_records/update.json.jbuilder @@ -0,0 +1 @@ +json.partial! 'api/v1/models/leave_record', formats: [:json], leave_record: @leave_record diff --git a/enterprise/app/views/api/v1/accounts/leaves/create.json.jbuilder b/enterprise/app/views/api/v1/accounts/leaves/create.json.jbuilder deleted file mode 100644 index 8f59e1a83..000000000 --- a/enterprise/app/views/api/v1/accounts/leaves/create.json.jbuilder +++ /dev/null @@ -1,3 +0,0 @@ -json.payload do - json.partial! 'api/v1/models/leave', formats: [:json], leave: @leave -end diff --git a/enterprise/app/views/api/v1/accounts/leaves/index.json.jbuilder b/enterprise/app/views/api/v1/accounts/leaves/index.json.jbuilder deleted file mode 100644 index a9d7a2b0e..000000000 --- a/enterprise/app/views/api/v1/accounts/leaves/index.json.jbuilder +++ /dev/null @@ -1,5 +0,0 @@ -json.payload do - json.array! @leaves do |leave| - json.partial! 'api/v1/models/leave', formats: [:json], leave: leave - end -end diff --git a/enterprise/app/views/api/v1/accounts/leaves/show.json.jbuilder b/enterprise/app/views/api/v1/accounts/leaves/show.json.jbuilder deleted file mode 100644 index 8f59e1a83..000000000 --- a/enterprise/app/views/api/v1/accounts/leaves/show.json.jbuilder +++ /dev/null @@ -1,3 +0,0 @@ -json.payload do - json.partial! 'api/v1/models/leave', formats: [:json], leave: @leave -end diff --git a/enterprise/app/views/api/v1/accounts/leaves/update.json.jbuilder b/enterprise/app/views/api/v1/accounts/leaves/update.json.jbuilder deleted file mode 100644 index 8f59e1a83..000000000 --- a/enterprise/app/views/api/v1/accounts/leaves/update.json.jbuilder +++ /dev/null @@ -1,3 +0,0 @@ -json.payload do - json.partial! 'api/v1/models/leave', formats: [:json], leave: @leave -end diff --git a/enterprise/app/views/api/v1/models/_leave.json.jbuilder b/enterprise/app/views/api/v1/models/_leave.json.jbuilder deleted file mode 100644 index 6c641c378..000000000 --- a/enterprise/app/views/api/v1/models/_leave.json.jbuilder +++ /dev/null @@ -1,26 +0,0 @@ -json.id leave.id -json.start_date leave.start_date -json.end_date leave.end_date -json.leave_type leave.leave_type -json.status leave.status -json.reason leave.reason -json.duration_in_days leave.duration_in_days -json.approved_at leave.approved_at&.to_i -json.created_at leave.created_at.to_i -json.updated_at leave.updated_at.to_i - -json.user do - json.id leave.user.id - json.name leave.user.name - json.email leave.user.email -end - -if leave.approver.present? - json.approver do - json.id leave.approver.id - json.name leave.approver.name - json.email leave.approver.email - end -else - json.approver nil -end diff --git a/enterprise/app/views/api/v1/models/_leave_record.json.jbuilder b/enterprise/app/views/api/v1/models/_leave_record.json.jbuilder new file mode 100644 index 000000000..80e427196 --- /dev/null +++ b/enterprise/app/views/api/v1/models/_leave_record.json.jbuilder @@ -0,0 +1,26 @@ +json.id leave_record.id +json.start_date leave_record.start_date +json.end_date leave_record.end_date +json.leave_type leave_record.leave_type +json.status leave_record.status +json.reason leave_record.reason +json.duration_in_days leave_record.duration_in_days +json.approved_at leave_record.approved_at&.to_i +json.created_at leave_record.created_at.to_i +json.updated_at leave_record.updated_at.to_i + +json.user do + json.id leave_record.user.id + json.name leave_record.user.name + json.email leave_record.user.email +end + +if leave_record.approver.present? + json.approver do + json.id leave_record.approver.id + json.name leave_record.approver.name + json.email leave_record.approver.email + end +else + json.approver nil +end diff --git a/spec/enterprise/controllers/api/v1/accounts/leave_records_controller_spec.rb b/spec/enterprise/controllers/api/v1/accounts/leave_records_controller_spec.rb new file mode 100644 index 000000000..220194977 --- /dev/null +++ b/spec/enterprise/controllers/api/v1/accounts/leave_records_controller_spec.rb @@ -0,0 +1,315 @@ +require 'rails_helper' + +RSpec.describe 'LeaveRecords API', type: :request do + let(:account) { create(:account) } + let(:administrator) { create(:user, account: account, role: :administrator) } + let(:agent) { create(:user, account: account, role: :agent) } + let(:other_user) { create(:user, account: account, role: :agent) } + + describe 'GET #index' do + let!(:agent_leave_record) { create(:leave_record, account: account, user: agent) } + let!(:other_user_leave_record) { create(:leave_record, account: account, user: other_user) } + + context 'when authenticated as administrator' do + it 'returns all leave records in the account' do + get "/api/v1/accounts/#{account.id}/leave_records", + headers: administrator.create_new_auth_token + + expect(response).to have_http_status(:success) + body = JSON.parse(response.body) + expect(body.length).to eq(2) + end + end + + context 'when authenticated as agent' do + it 'returns only own leave records' do + get "/api/v1/accounts/#{account.id}/leave_records", + headers: agent.create_new_auth_token + + expect(response).to have_http_status(:success) + body = JSON.parse(response.body) + expect(body.length).to eq(1) + expect(body[0]['user']['id']).to eq(agent.id) + end + end + + context 'when unauthenticated' do + it 'returns unauthorized' do + get "/api/v1/accounts/#{account.id}/leave_records" + + expect(response).to have_http_status(:unauthorized) + end + end + end + + describe 'GET #show' do + let(:leave_record) { create(:leave_record, account: account, user: agent) } + let(:other_leave_record) { create(:leave_record, account: account, user: other_user) } + + context 'when authenticated as administrator' do + it 'shows any leave record in the account' do + get "/api/v1/accounts/#{account.id}/leave_records/#{other_leave_record.id}", + headers: administrator.create_new_auth_token + + expect(response).to have_http_status(:success) + body = JSON.parse(response.body) + expect(body['id']).to eq(other_leave_record.id) + expect(body['user']['id']).to eq(other_user.id) + end + end + + context 'when authenticated as agent' do + it 'shows own leave record' do + get "/api/v1/accounts/#{account.id}/leave_records/#{leave_record.id}", + headers: agent.create_new_auth_token + + expect(response).to have_http_status(:success) + body = JSON.parse(response.body) + expect(body['id']).to eq(leave_record.id) + expect(body['user']['id']).to eq(agent.id) + end + + it 'denies access to other user leave record' do + get "/api/v1/accounts/#{account.id}/leave_records/#{other_leave_record.id}", + headers: agent.create_new_auth_token + + expect(response).to have_http_status(:not_found) + end + end + end + + describe 'POST #create' do + let(:valid_params) do + { + leave_record: { + start_date: 1.week.from_now.to_date, + end_date: 2.weeks.from_now.to_date, + leave_type: 'annual', + reason: 'Vacation time' + } + } + end + + context 'when authenticated as agent' do + it 'creates a leave record for the current user' do + expect do + post "/api/v1/accounts/#{account.id}/leave_records", + params: valid_params, + headers: agent.create_new_auth_token + end.to change(LeaveRecord, :count).by(1) + + expect(response).to have_http_status(:success) + body = JSON.parse(response.body) + expect(body['user']['id']).to eq(agent.id) + expect(body['leave_type']).to eq('annual') + expect(body['status']).to eq('pending') + end + + it 'returns validation errors for invalid data' do + invalid_params = valid_params.deep_merge( + leave_record: { start_date: nil } + ) + + post "/api/v1/accounts/#{account.id}/leave_records", + params: invalid_params, + headers: agent.create_new_auth_token + + expect(response).to have_http_status(:unprocessable_entity) + end + end + + context 'when authenticated as administrator' do + it 'creates a leave record for the current user' do + expect do + post "/api/v1/accounts/#{account.id}/leave_records", + params: valid_params, + headers: administrator.create_new_auth_token + end.to change(LeaveRecord, :count).by(1) + + expect(response).to have_http_status(:success) + body = JSON.parse(response.body) + expect(body['user']['id']).to eq(administrator.id) + end + end + end + + describe 'PUT #update' do + let(:leave_record) { create(:leave_record, account: account, user: agent, status: :pending) } + let(:other_leave_record) { create(:leave_record, account: account, user: other_user, status: :pending) } + let(:update_params) do + { + leave_record: { + reason: 'Updated reason' + } + } + end + + context 'when authenticated as administrator' do + it 'updates any pending leave record in the account' do + put "/api/v1/accounts/#{account.id}/leave_records/#{other_leave_record.id}", + params: update_params, + headers: administrator.create_new_auth_token + + expect(response).to have_http_status(:success) + body = JSON.parse(response.body) + expect(body['reason']).to eq('Updated reason') + end + end + + context 'when authenticated as agent' do + it 'updates own pending leave record' do + put "/api/v1/accounts/#{account.id}/leave_records/#{leave_record.id}", + params: update_params, + headers: agent.create_new_auth_token + + expect(response).to have_http_status(:success) + body = JSON.parse(response.body) + expect(body['reason']).to eq('Updated reason') + end + + it 'denies updating other user leave record' do + put "/api/v1/accounts/#{account.id}/leave_records/#{other_leave_record.id}", + params: update_params, + headers: agent.create_new_auth_token + + expect(response).to have_http_status(:not_found) + end + + it 'denies updating approved leave record' do + approved_leave_record = create(:leave_record, account: account, user: agent, status: :approved) + + put "/api/v1/accounts/#{account.id}/leave_records/#{approved_leave_record.id}", + params: update_params, + headers: agent.create_new_auth_token + + expect(response).to have_http_status(:not_found) + end + end + end + + describe 'DELETE #destroy' do + let(:leave_record) { create(:leave_record, account: account, user: agent, status: :pending) } + let(:other_leave_record) { create(:leave_record, account: account, user: other_user, status: :pending) } + + context 'when authenticated as administrator' do + it 'deletes any cancellable leave record in the account' do + expect do + delete "/api/v1/accounts/#{account.id}/leave_records/#{other_leave_record.id}", + headers: administrator.create_new_auth_token + end.to change(LeaveRecord, :count).by(-1) + + expect(response).to have_http_status(:ok) + end + end + + context 'when authenticated as agent' do + it 'deletes own cancellable leave record' do + expect do + delete "/api/v1/accounts/#{account.id}/leave_records/#{leave_record.id}", + headers: agent.create_new_auth_token + end.to change(LeaveRecord, :count).by(-1) + + expect(response).to have_http_status(:ok) + end + + it 'denies deleting other user leave record' do + delete "/api/v1/accounts/#{account.id}/leave_records/#{other_leave_record.id}", + headers: agent.create_new_auth_token + + expect(response).to have_http_status(:not_found) + end + + it 'denies deleting non-cancellable leave record' do + non_cancellable_leave_record = create(:leave_record, account: account, user: agent, + status: :approved, + start_date: Date.current, + end_date: Date.current + 2.days) + + delete "/api/v1/accounts/#{account.id}/leave_records/#{non_cancellable_leave_record.id}", + headers: agent.create_new_auth_token + + expect(response).to have_http_status(:not_found) + end + end + end + + describe 'PATCH #approve' do + let(:leave_record) { create(:leave_record, account: account, user: agent, status: :pending) } + + context 'when authenticated as administrator' do + it 'approves the leave record' do + patch "/api/v1/accounts/#{account.id}/leave_records/#{leave_record.id}/approve", + headers: administrator.create_new_auth_token + + expect(response).to have_http_status(:success) + body = JSON.parse(response.body) + expect(body['status']).to eq('approved') + expect(body['approver']['id']).to eq(administrator.id) + end + end + + context 'when authenticated as agent' do + it 'denies approval' do + patch "/api/v1/accounts/#{account.id}/leave_records/#{leave_record.id}/approve", + headers: agent.create_new_auth_token + + expect(response).to have_http_status(:not_found) + end + end + end + + describe 'PATCH #reject' do + let(:leave_record) { create(:leave_record, account: account, user: agent, status: :pending) } + + context 'when authenticated as administrator' do + it 'rejects the leave record' do + patch "/api/v1/accounts/#{account.id}/leave_records/#{leave_record.id}/reject", + headers: administrator.create_new_auth_token + + expect(response).to have_http_status(:success) + body = JSON.parse(response.body) + expect(body['status']).to eq('rejected') + expect(body['approver']['id']).to eq(administrator.id) + end + end + + context 'when authenticated as agent' do + it 'denies rejection' do + patch "/api/v1/accounts/#{account.id}/leave_records/#{leave_record.id}/reject", + headers: agent.create_new_auth_token + + expect(response).to have_http_status(:not_found) + end + end + end + + describe 'response format' do + let(:leave_record) { create(:leave_record, account: account, user: agent) } + + it 'returns leave record data in correct format' do + get "/api/v1/accounts/#{account.id}/leave_records/#{leave_record.id}", + headers: agent.create_new_auth_token + + expect(response).to have_http_status(:success) + body = JSON.parse(response.body) + + expect(body.keys).to include('id', 'start_date', 'end_date', 'leave_type', 'status', + 'reason', 'duration_in_days', 'created_at', 'updated_at', 'user') + expect(body['user'].keys).to include('id', 'name', 'email') + end + + it 'includes approver data when present' do + approved_leave_record = create(:leave_record, account: account, user: agent, status: :approved) + approved_leave_record.update!(approver: administrator, approved_at: Time.current) + + get "/api/v1/accounts/#{account.id}/leave_records/#{approved_leave_record.id}", + headers: agent.create_new_auth_token + + expect(response).to have_http_status(:success) + body = JSON.parse(response.body) + + expect(body['approver']).not_to be_nil + expect(body['approver'].keys).to include('id', 'name', 'email') + end + end +end \ No newline at end of file diff --git a/spec/enterprise/controllers/api/v1/accounts/leaves_controller_spec.rb b/spec/enterprise/controllers/api/v1/accounts/leaves_controller_spec.rb deleted file mode 100644 index e9a6abea2..000000000 --- a/spec/enterprise/controllers/api/v1/accounts/leaves_controller_spec.rb +++ /dev/null @@ -1,550 +0,0 @@ -require 'rails_helper' - -RSpec.describe 'Leaves API', type: :request do - let(:account) { create(:account) } - let(:administrator) { create(:user, account: account, role: :administrator) } - let(:agent) { create(:user, account: account, role: :agent) } - let(:other_user) { create(:user, account: account, role: :agent) } - - describe 'GET #index' do - before do - create(:leave, account: account, user: agent) - create(:leave, account: account, user: other_user) - end - - context 'when authenticated as administrator' do - it 'returns all leaves in the account' do - get "/api/v1/accounts/#{account.id}/leaves", - headers: administrator.create_new_auth_token - - expect(response).to have_http_status(:success) - body = JSON.parse(response.body) - expect(body['leaves'].length).to eq(2) - end - end - - context 'when authenticated as agent' do - it 'returns only own leaves' do - get "/api/v1/accounts/#{account.id}/leaves", - headers: agent.create_new_auth_token - - expect(response).to have_http_status(:success) - body = JSON.parse(response.body) - expect(body['leaves'].length).to eq(1) - expect(body['leaves'][0]['user']['id']).to eq(agent.id) - end - end - - context 'when unauthenticated' do - it 'returns unauthorized' do - get "/api/v1/accounts/#{account.id}/leaves" - - expect(response).to have_http_status(:unauthorized) - end - end - end - - describe 'GET #show' do - let(:leave) { create(:leave, account: account, user: agent) } - let(:other_leave) { create(:leave, account: account, user: other_user) } - - context 'when authenticated as administrator' do - it 'shows any leave in the account' do - get "/api/v1/accounts/#{account.id}/leaves/#{leave.id}", - headers: administrator.create_new_auth_token - - expect(response).to have_http_status(:success) - body = JSON.parse(response.body) - expect(body['id']).to eq(leave.id) - end - end - - context 'when authenticated as agent viewing own leave' do - it 'shows the leave' do - get "/api/v1/accounts/#{account.id}/leaves/#{leave.id}", - headers: agent.create_new_auth_token - - expect(response).to have_http_status(:success) - body = JSON.parse(response.body) - expect(body['id']).to eq(leave.id) - expect(body['user']['id']).to eq(agent.id) - end - end - - context 'when authenticated as agent viewing other user leave' do - it 'returns unauthorized' do - get "/api/v1/accounts/#{account.id}/leaves/#{other_leave.id}", - headers: agent.create_new_auth_token - - expect(response).to have_http_status(:unauthorized) - end - end - - context 'when leave does not exist' do - it 'returns not found' do - get "/api/v1/accounts/#{account.id}/leaves/99999", - headers: administrator.create_new_auth_token - - expect(response).to have_http_status(:not_found) - end - end - - context 'when unauthenticated' do - it 'returns unauthorized' do - get "/api/v1/accounts/#{account.id}/leaves/#{leave.id}" - - expect(response).to have_http_status(:unauthorized) - end - end - end - - describe 'POST #create' do - let(:valid_params) do - { - leave: { - start_date: 1.week.from_now.to_date, - end_date: 2.weeks.from_now.to_date, - leave_type: 'annual', - reason: 'Annual vacation' - } - } - end - - context 'when authenticated as administrator' do - it 'creates a leave for current user' do - expect do - post "/api/v1/accounts/#{account.id}/leaves", - params: valid_params, - headers: administrator.create_new_auth_token - end.to change(Leave, :count).by(1) - - expect(response).to have_http_status(:success) - body = JSON.parse(response.body) - expect(body['leave_type']).to eq('annual') - expect(body['user']['id']).to eq(administrator.id) - end - - it 'creates leave with other type' do - params = valid_params.merge(leave: valid_params[:leave].merge(leave_type: 'other')) - - post "/api/v1/accounts/#{account.id}/leaves", - params: params, - headers: administrator.create_new_auth_token - - expect(response).to have_http_status(:success) - body = JSON.parse(response.body) - expect(body['leave_type']).to eq('other') - end - end - - context 'when authenticated as agent' do - it 'creates a leave for current user' do - expect do - post "/api/v1/accounts/#{account.id}/leaves", - params: valid_params, - headers: agent.create_new_auth_token - end.to change(Leave, :count).by(1) - - expect(response).to have_http_status(:success) - body = JSON.parse(response.body) - expect(body['user']['id']).to eq(agent.id) - end - end - - context 'with invalid parameters' do - it 'returns validation errors for missing start date' do - invalid_params = valid_params.merge(leave: valid_params[:leave].except(:start_date)) - - post "/api/v1/accounts/#{account.id}/leaves", - params: invalid_params, - headers: agent.create_new_auth_token - - expect(response).to have_http_status(:unprocessable_entity) - end - - it 'returns validation errors for end date before start date' do - invalid_params = valid_params.merge( - leave: valid_params[:leave].merge( - start_date: 2.weeks.from_now.to_date, - end_date: 1.week.from_now.to_date - ) - ) - - post "/api/v1/accounts/#{account.id}/leaves", - params: invalid_params, - headers: agent.create_new_auth_token - - expect(response).to have_http_status(:unprocessable_entity) - end - - it 'returns validation errors for past start date on pending leave' do - invalid_params = valid_params.merge( - leave: valid_params[:leave].merge( - start_date: 1.week.ago.to_date, - end_date: Date.current - ) - ) - - post "/api/v1/accounts/#{account.id}/leaves", - params: invalid_params, - headers: agent.create_new_auth_token - - expect(response).to have_http_status(:unprocessable_entity) - end - end - - context 'when unauthenticated' do - it 'returns unauthorized' do - post "/api/v1/accounts/#{account.id}/leaves", - params: valid_params - - expect(response).to have_http_status(:unauthorized) - end - end - end - - describe 'PUT #update' do - let(:leave) { create(:leave, account: account, user: agent, status: :pending) } - let(:other_leave) { create(:leave, account: account, user: other_user, status: :pending) } - let(:approved_leave) { create(:leave, account: account, user: agent, status: :approved) } - - let(:update_params) do - { - leave: { - reason: 'Updated reason' - } - } - end - - context 'when authenticated as administrator' do - it 'updates any pending leave' do - put "/api/v1/accounts/#{account.id}/leaves/#{leave.id}", - params: update_params, - headers: administrator.create_new_auth_token - - expect(response).to have_http_status(:success) - body = JSON.parse(response.body) - expect(body['reason']).to eq('Updated reason') - end - - it 'cannot update non-pending leaves' do - put "/api/v1/accounts/#{account.id}/leaves/#{approved_leave.id}", - params: update_params, - headers: administrator.create_new_auth_token - - expect(response).to have_http_status(:unauthorized) - end - end - - context 'when authenticated as agent' do - it 'updates own pending leave' do - put "/api/v1/accounts/#{account.id}/leaves/#{leave.id}", - params: update_params, - headers: agent.create_new_auth_token - - expect(response).to have_http_status(:success) - body = JSON.parse(response.body) - expect(body['reason']).to eq('Updated reason') - end - - it 'cannot update other user leave' do - put "/api/v1/accounts/#{account.id}/leaves/#{other_leave.id}", - params: update_params, - headers: agent.create_new_auth_token - - expect(response).to have_http_status(:unauthorized) - end - - it 'cannot update non-pending leaves' do - put "/api/v1/accounts/#{account.id}/leaves/#{approved_leave.id}", - params: update_params, - headers: administrator.create_new_auth_token - - expect(response).to have_http_status(:unauthorized) - end - end - - context 'when leave does not exist' do - it 'returns not found' do - put "/api/v1/accounts/#{account.id}/leaves/99999", - params: update_params, - headers: administrator.create_new_auth_token - - expect(response).to have_http_status(:not_found) - end - end - - context 'when unauthenticated' do - it 'returns unauthorized' do - put "/api/v1/accounts/#{account.id}/leaves/#{leave.id}", - params: update_params - - expect(response).to have_http_status(:unauthorized) - end - end - end - - describe 'DELETE #destroy' do - let(:pending_leave) { create(:leave, account: account, user: agent, status: :pending) } - let(:approved_future_leave) do - create(:leave, account: account, user: agent, status: :approved, - start_date: 1.month.from_now.to_date, end_date: 1.month.from_now.to_date + 5.days) - end - let(:approved_past_leave) do - create(:leave, account: account, user: agent, status: :approved, - start_date: 1.week.ago.to_date, end_date: 3.days.ago.to_date) - end - let(:other_leave) { create(:leave, account: account, user: other_user, status: :pending) } - - context 'when authenticated as administrator' do - it 'deletes cancellable leaves' do - expect do - delete "/api/v1/accounts/#{account.id}/leaves/#{pending_leave.id}", - headers: administrator.create_new_auth_token - end.to change(Leave, :count).by(-1) - - expect(response).to have_http_status(:success) - end - - it 'deletes approved future leaves' do - expect do - delete "/api/v1/accounts/#{account.id}/leaves/#{approved_future_leave.id}", - headers: administrator.create_new_auth_token - end.to change(Leave, :count).by(-1) - - expect(response).to have_http_status(:success) - end - - it 'cannot delete non-cancellable leaves' do - delete "/api/v1/accounts/#{account.id}/leaves/#{approved_past_leave.id}", - headers: administrator.create_new_auth_token - - expect(response).to have_http_status(:unauthorized) - end - end - - context 'when authenticated as agent' do - it 'deletes own cancellable leaves' do - expect do - delete "/api/v1/accounts/#{account.id}/leaves/#{pending_leave.id}", - headers: agent.create_new_auth_token - end.to change(Leave, :count).by(-1) - - expect(response).to have_http_status(:success) - end - - it 'cannot delete other user leaves' do - delete "/api/v1/accounts/#{account.id}/leaves/#{other_leave.id}", - headers: agent.create_new_auth_token - - expect(response).to have_http_status(:unauthorized) - end - end - - context 'when leave does not exist' do - it 'returns not found' do - delete "/api/v1/accounts/#{account.id}/leaves/99999", - headers: administrator.create_new_auth_token - - expect(response).to have_http_status(:not_found) - end - end - - context 'when unauthenticated' do - it 'returns unauthorized' do - delete "/api/v1/accounts/#{account.id}/leaves/#{pending_leave.id}" - - expect(response).to have_http_status(:unauthorized) - end - end - end - - describe 'PATCH #approve' do - let(:pending_leave) { create(:leave, account: account, user: agent, status: :pending) } - let(:approved_leave) { create(:leave, account: account, user: agent, status: :approved) } - - context 'when authenticated as administrator' do - it 'approves pending leave' do - patch "/api/v1/accounts/#{account.id}/leaves/#{pending_leave.id}/approve", - headers: administrator.create_new_auth_token - - expect(response).to have_http_status(:success) - body = JSON.parse(response.body) - expect(body['status']).to eq('approved') - expect(body['approver']['id']).to eq(administrator.id) - expect(body['approved_at']).not_to be_nil - - pending_leave.reload - expect(pending_leave.status).to eq('approved') - expect(pending_leave.approver).to eq(administrator) - end - - it 'updates approved_at timestamp' do - freeze_time = Time.current - allow(Time).to receive(:current).and_return(freeze_time) - - patch "/api/v1/accounts/#{account.id}/leaves/#{pending_leave.id}/approve", - headers: administrator.create_new_auth_token - - expect(response).to have_http_status(:success) - pending_leave.reload - expect(pending_leave.approved_at.to_i).to eq(freeze_time.to_i) - end - end - - context 'when authenticated as agent' do - it 'returns unauthorized' do - patch "/api/v1/accounts/#{account.id}/leaves/#{pending_leave.id}/approve", - headers: agent.create_new_auth_token - - expect(response).to have_http_status(:unauthorized) - end - end - - context 'when leave does not exist' do - it 'returns not found' do - patch "/api/v1/accounts/#{account.id}/leaves/99999/approve", - headers: administrator.create_new_auth_token - - expect(response).to have_http_status(:not_found) - end - end - - context 'when unauthenticated' do - it 'returns unauthorized' do - patch "/api/v1/accounts/#{account.id}/leaves/#{pending_leave.id}/approve" - - expect(response).to have_http_status(:unauthorized) - end - end - end - - describe 'PATCH #reject' do - let(:pending_leave) { create(:leave, account: account, user: agent, status: :pending) } - let(:rejected_leave) { create(:leave, account: account, user: agent, status: :rejected) } - - context 'when authenticated as administrator' do - it 'rejects pending leave' do - patch "/api/v1/accounts/#{account.id}/leaves/#{pending_leave.id}/reject", - headers: administrator.create_new_auth_token - - expect(response).to have_http_status(:success) - body = JSON.parse(response.body) - expect(body['status']).to eq('rejected') - expect(body['approver']['id']).to eq(administrator.id) - expect(body['approved_at']).not_to be_nil - - pending_leave.reload - expect(pending_leave.status).to eq('rejected') - expect(pending_leave.approver).to eq(administrator) - end - - it 'updates approved_at timestamp on rejection' do - freeze_time = Time.current - allow(Time).to receive(:current).and_return(freeze_time) - - patch "/api/v1/accounts/#{account.id}/leaves/#{pending_leave.id}/reject", - headers: administrator.create_new_auth_token - - expect(response).to have_http_status(:success) - pending_leave.reload - expect(pending_leave.approved_at.to_i).to eq(freeze_time.to_i) - end - end - - context 'when authenticated as agent' do - it 'returns unauthorized' do - patch "/api/v1/accounts/#{account.id}/leaves/#{pending_leave.id}/reject", - headers: agent.create_new_auth_token - - expect(response).to have_http_status(:unauthorized) - end - end - - context 'when leave does not exist' do - it 'returns not found' do - patch "/api/v1/accounts/#{account.id}/leaves/99999/reject", - headers: administrator.create_new_auth_token - - expect(response).to have_http_status(:not_found) - end - end - - context 'when unauthenticated' do - it 'returns unauthorized' do - patch "/api/v1/accounts/#{account.id}/leaves/#{pending_leave.id}/reject" - - expect(response).to have_http_status(:unauthorized) - end - end - end - - describe 'Business logic scenarios' do - describe 'leave overlap detection' do - let(:existing_leave) do - create(:leave, account: account, user: agent, status: :approved, - start_date: 1.month.from_now.to_date, - end_date: 1.month.from_now.to_date + 5.days) - end - - it 'allows non-overlapping leaves' do - existing_leave - - params = { - leave: { - start_date: 2.months.from_now.to_date, - end_date: 2.months.from_now.to_date + 3.days, - leave_type: 'annual', - reason: 'Second vacation' - } - } - - post "/api/v1/accounts/#{account.id}/leaves", - params: params, - headers: agent.create_new_auth_token - - expect(response).to have_http_status(:success) - end - end - - describe 'leave duration calculation' do - it 'includes duration in response' do - params = { - leave: { - start_date: 1.week.from_now.to_date, - end_date: 1.week.from_now.to_date + 4.days, - leave_type: 'annual', - reason: 'Five day vacation' - } - } - - post "/api/v1/accounts/#{account.id}/leaves", - params: params, - headers: agent.create_new_auth_token - - expect(response).to have_http_status(:success) - body = JSON.parse(response.body) - expect(body['duration_in_days']).to eq(5) - end - end - - describe 'approver validation' do - let(:non_admin_approver) { create(:user, account: account, role: :agent) } - - before do - # Ensure approver is linked to account - create(:account_user, account: account, user: non_admin_approver, role: :agent) - create(:account_user, account: account, user: administrator, role: :administrator) - end - - it 'rejects leave approval by non-administrator' do - leave = create(:leave, account: account, user: agent, status: :pending) - - # Simulate manual update with non-admin approver (would fail validation) - expect do - leave.update!(approved_by_id: non_admin_approver.id, status: :approved) - end.to raise_error(ActiveRecord::RecordInvalid) - end - end - end -end diff --git a/spec/enterprise/models/leave_record_spec.rb b/spec/enterprise/models/leave_record_spec.rb new file mode 100644 index 000000000..79f03e670 --- /dev/null +++ b/spec/enterprise/models/leave_record_spec.rb @@ -0,0 +1,287 @@ +require 'rails_helper' + +RSpec.describe LeaveRecord, type: :model do + let(:account) { create(:account) } + let(:user) { create(:user, account: account) } + let(:approver) { create(:user, :administrator, account: account) } + + describe '#duration_in_days' do + it 'calculates duration correctly for single day' do + leave_record = create(:leave_record, start_date: Date.current, end_date: Date.current, status: :approved) + expect(leave_record.duration_in_days).to eq(1) + end + + it 'calculates duration correctly for multiple days' do + leave_record = create(:leave_record, start_date: Date.current, end_date: Date.current + 4.days, status: :approved) + expect(leave_record.duration_in_days).to eq(5) + end + + it 'returns 0 when start_date is nil' do + leave_record = build(:leave_record, start_date: nil, end_date: Date.current) + expect(leave_record.duration_in_days).to eq(0) + end + + it 'returns 0 when end_date is nil' do + leave_record = build(:leave_record, start_date: Date.current, end_date: nil) + expect(leave_record.duration_in_days).to eq(0) + end + end + + describe '#can_be_cancelled?' do + context 'when leave record is pending' do + it 'returns true' do + leave_record = create(:leave_record, status: :pending) + expect(leave_record.can_be_cancelled?).to be true + end + end + + context 'when leave record is approved' do + it 'returns true for future leaves' do + leave_record = create(:leave_record, status: :approved, + start_date: 1.week.from_now.to_date, + end_date: 2.weeks.from_now.to_date) + expect(leave_record.can_be_cancelled?).to be true + end + + it 'returns false for current leaves' do + leave_record = create(:leave_record, status: :approved, + start_date: Date.current, + end_date: Date.current + 1.day) + expect(leave_record.can_be_cancelled?).to be false + end + + it 'returns false for past leaves' do + leave_record = create(:leave_record, status: :approved, + start_date: 1.week.ago.to_date, + end_date: 3.days.ago.to_date) + expect(leave_record.can_be_cancelled?).to be false + end + end + + context 'when leave record is rejected' do + it 'returns false' do + leave_record = create(:leave_record, status: :rejected) + expect(leave_record.can_be_cancelled?).to be false + end + end + + context 'when leave record is cancelled' do + it 'returns false' do + leave_record = create(:leave_record, status: :cancelled) + expect(leave_record.can_be_cancelled?).to be false + end + end + end + + describe '#overlaps_with?' do + let(:base_leave_record) do + create(:leave_record, start_date: Date.current + 10.days, + end_date: Date.current + 15.days) + end + + it 'returns false for non-LeaveRecord objects' do + expect(base_leave_record.overlaps_with?('not a leave record')).to be false + end + + it 'detects overlapping leave records - same dates' do + overlapping_leave_record = build(:leave_record, start_date: Date.current + 10.days, + end_date: Date.current + 15.days) + expect(base_leave_record.overlaps_with?(overlapping_leave_record)).to be true + end + + it 'detects overlapping leave records - partial overlap start' do + overlapping_leave_record = build(:leave_record, start_date: Date.current + 8.days, + end_date: Date.current + 12.days) + expect(base_leave_record.overlaps_with?(overlapping_leave_record)).to be true + end + + it 'detects overlapping leave records - partial overlap end' do + overlapping_leave_record = build(:leave_record, start_date: Date.current + 13.days, + end_date: Date.current + 18.days) + expect(base_leave_record.overlaps_with?(overlapping_leave_record)).to be true + end + + it 'detects overlapping leave records - contained within' do + overlapping_leave_record = build(:leave_record, start_date: Date.current + 12.days, + end_date: Date.current + 13.days) + expect(base_leave_record.overlaps_with?(overlapping_leave_record)).to be true + end + + it 'detects overlapping leave records - contains other' do + overlapping_leave_record = build(:leave_record, start_date: Date.current + 8.days, + end_date: Date.current + 18.days) + expect(base_leave_record.overlaps_with?(overlapping_leave_record)).to be true + end + + it 'returns false for non-overlapping leave records - before' do + non_overlapping_leave_record = build(:leave_record, start_date: Date.current + 5.days, + end_date: Date.current + 9.days) + expect(base_leave_record.overlaps_with?(non_overlapping_leave_record)).to be false + end + + it 'returns false for non-overlapping leave records - after' do + non_overlapping_leave_record = build(:leave_record, start_date: Date.current + 16.days, + end_date: Date.current + 20.days) + expect(base_leave_record.overlaps_with?(non_overlapping_leave_record)).to be false + end + + it 'returns false for adjacent leave records - ending where other starts' do + adjacent_leave_record = build(:leave_record, start_date: Date.current + 16.days, + end_date: Date.current + 20.days) + expect(base_leave_record.overlaps_with?(adjacent_leave_record)).to be false + end + + it 'returns false for adjacent leave records - starting where other ends' do + adjacent_leave_record = build(:leave_record, start_date: Date.current + 5.days, + end_date: Date.current + 9.days) + expect(base_leave_record.overlaps_with?(adjacent_leave_record)).to be false + end + end + + describe '#approve!' do + let(:leave_record) { create(:leave_record, account: account, status: :pending) } + + before do + # Ensure approver has administrator role in the account + account_user = approver.account_users.find_by(account: account) + account_user.update!(role: :administrator) if account_user + end + + it 'updates status to approved' do + leave_record.approve!(approver) + expect(leave_record.status).to eq('approved') + end + + it 'sets approver' do + leave_record.approve!(approver) + expect(leave_record.approver).to eq(approver) + end + + it 'sets approved_at timestamp' do + freeze_time = Time.current + allow(Time).to receive(:current).and_return(freeze_time) + + leave_record.approve!(approver) + expect(leave_record.approved_at.to_i).to eq(freeze_time.to_i) + end + + it 'raises error if approver is not admin' do + non_admin = create(:user, account: account, role: :agent) + + expect do + leave_record.approve!(non_admin) + end.to raise_error(ActiveRecord::RecordInvalid, /must be an administrator/) + end + end + + describe '#reject!' do + let(:leave_record) { create(:leave_record, account: account, status: :pending) } + + before do + # Ensure approver has administrator role in the account + account_user = approver.account_users.find_by(account: account) + account_user.update!(role: :administrator) if account_user + end + + it 'updates status to rejected' do + leave_record.reject!(approver) + expect(leave_record.status).to eq('rejected') + end + + it 'sets approver' do + leave_record.reject!(approver) + expect(leave_record.approver).to eq(approver) + end + + it 'sets approved_at timestamp' do + freeze_time = Time.current + allow(Time).to receive(:current).and_return(freeze_time) + + leave_record.reject!(approver) + expect(leave_record.approved_at.to_i).to eq(freeze_time.to_i) + end + + it 'raises error if approver is not admin' do + non_admin = create(:user, account: account, role: :agent) + + expect do + leave_record.reject!(non_admin) + end.to raise_error(ActiveRecord::RecordInvalid, /must be an administrator/) + end + end + + describe 'custom validations' do + describe '#end_date_after_start_date' do + it 'is valid when end_date is after start_date' do + leave_record = build(:leave_record, start_date: 1.day.from_now.to_date, end_date: 2.days.from_now.to_date) + expect(leave_record).to be_valid + end + + it 'is valid when end_date equals start_date' do + leave_record = build(:leave_record, start_date: 1.day.from_now.to_date, end_date: 1.day.from_now.to_date) + expect(leave_record).to be_valid + end + + it 'is invalid when end_date is before start_date' do + leave_record = build(:leave_record, start_date: 2.days.from_now.to_date, end_date: 1.day.from_now.to_date) + expect(leave_record).not_to be_valid + expect(leave_record.errors[:end_date]).to include('must be after start date') + end + + it 'skips validation when dates are nil' do + leave_record = build(:leave_record, start_date: nil, end_date: nil) + leave_record.valid? + expect(leave_record.errors[:end_date]).not_to include('must be after start date') + end + end + + describe '#future_dates_for_pending_leaves' do + it 'is valid for future start_date on pending leave record' do + leave_record = build(:leave_record, status: :pending, start_date: Date.current + 1.day) + expect(leave_record).to be_valid + end + + it 'is invalid for current start_date on pending leave record' do + leave_record = build(:leave_record, status: :pending, start_date: Date.current) + expect(leave_record).not_to be_valid + expect(leave_record.errors[:start_date]).to include('must be in the future') + end + + it 'is invalid for past start_date on pending leave record' do + leave_record = build(:leave_record, status: :pending, start_date: Date.current - 1.day) + expect(leave_record).not_to be_valid + expect(leave_record.errors[:start_date]).to include('must be in the future') + end + + it 'skips validation for non-pending leave records' do + leave_record = build(:leave_record, status: :approved, start_date: Date.current - 1.day) + leave_record.valid? + expect(leave_record.errors[:start_date]).not_to include('must be in the future') + end + end + + describe '#approver_is_admin' do + let(:admin_user) { create(:user, :administrator, account: account) } + let(:agent_user) { create(:user, account: account, role: :agent) } + + # Users are already associated with the account via factory + + it 'is valid with admin approver' do + leave_record = build(:leave_record, account: account, approver: admin_user, status: :approved) + expect(leave_record).to be_valid + end + + it 'is invalid with non-admin approver' do + leave_record = build(:leave_record, account: account, approver: agent_user, status: :approved) + expect(leave_record).not_to be_valid + expect(leave_record.errors[:approved_by]).to include('must be an administrator') + end + + it 'skips validation when approver is nil' do + leave_record = build(:leave_record, account: account, approver: nil) + leave_record.valid? + expect(leave_record.errors[:approved_by]).not_to include('must be an administrator') + end + end + end +end \ No newline at end of file diff --git a/spec/enterprise/models/leave_spec.rb b/spec/enterprise/models/leave_spec.rb deleted file mode 100644 index 9793a9c2e..000000000 --- a/spec/enterprise/models/leave_spec.rb +++ /dev/null @@ -1,288 +0,0 @@ -require 'rails_helper' - -RSpec.describe Leave, type: :model do - let(:account) { create(:account) } - let(:user) { create(:user, account: account) } - let(:approver) { create(:user, account: account, role: :administrator) } - - describe '#duration_in_days' do - it 'calculates duration correctly for single day' do - leave = create(:leave, start_date: Date.current, end_date: Date.current) - expect(leave.duration_in_days).to eq(1) - end - - it 'calculates duration correctly for multiple days' do - leave = create(:leave, start_date: Date.current, end_date: Date.current + 4.days) - expect(leave.duration_in_days).to eq(5) - end - - it 'returns 0 when start_date is nil' do - leave = build(:leave, start_date: nil, end_date: Date.current) - expect(leave.duration_in_days).to eq(0) - end - - it 'returns 0 when end_date is nil' do - leave = build(:leave, start_date: Date.current, end_date: nil) - expect(leave.duration_in_days).to eq(0) - end - end - - describe '#can_be_cancelled?' do - context 'when leave is pending' do - it 'returns true' do - leave = create(:leave, status: :pending) - expect(leave.can_be_cancelled?).to be true - end - end - - context 'when leave is approved' do - it 'returns true for future leaves' do - leave = create(:leave, status: :approved, - start_date: 1.week.from_now.to_date, - end_date: 2.weeks.from_now.to_date) - expect(leave.can_be_cancelled?).to be true - end - - it 'returns false for current leaves' do - leave = create(:leave, status: :approved, - start_date: Date.current, - end_date: Date.current + 1.day) - expect(leave.can_be_cancelled?).to be false - end - - it 'returns false for past leaves' do - leave = create(:leave, status: :approved, - start_date: 1.week.ago.to_date, - end_date: 3.days.ago.to_date) - expect(leave.can_be_cancelled?).to be false - end - end - - context 'when leave is rejected' do - it 'returns false' do - leave = create(:leave, status: :rejected) - expect(leave.can_be_cancelled?).to be false - end - end - - context 'when leave is cancelled' do - it 'returns false' do - leave = create(:leave, status: :cancelled) - expect(leave.can_be_cancelled?).to be false - end - end - end - - describe '#overlaps_with?' do - let(:base_leave) do - create(:leave, start_date: Date.current + 10.days, - end_date: Date.current + 15.days) - end - - it 'returns false for non-Leave objects' do - expect(base_leave.overlaps_with?('not a leave')).to be false - end - - it 'detects overlapping leaves - same dates' do - overlapping_leave = build(:leave, start_date: Date.current + 10.days, - end_date: Date.current + 15.days) - expect(base_leave.overlaps_with?(overlapping_leave)).to be true - end - - it 'detects overlapping leaves - partial overlap start' do - overlapping_leave = build(:leave, start_date: Date.current + 8.days, - end_date: Date.current + 12.days) - expect(base_leave.overlaps_with?(overlapping_leave)).to be true - end - - it 'detects overlapping leaves - partial overlap end' do - overlapping_leave = build(:leave, start_date: Date.current + 13.days, - end_date: Date.current + 18.days) - expect(base_leave.overlaps_with?(overlapping_leave)).to be true - end - - it 'detects overlapping leaves - contained within' do - overlapping_leave = build(:leave, start_date: Date.current + 12.days, - end_date: Date.current + 13.days) - expect(base_leave.overlaps_with?(overlapping_leave)).to be true - end - - it 'detects overlapping leaves - contains other' do - overlapping_leave = build(:leave, start_date: Date.current + 8.days, - end_date: Date.current + 18.days) - expect(base_leave.overlaps_with?(overlapping_leave)).to be true - end - - it 'returns false for non-overlapping leaves - before' do - non_overlapping_leave = build(:leave, start_date: Date.current + 5.days, - end_date: Date.current + 9.days) - expect(base_leave.overlaps_with?(non_overlapping_leave)).to be false - end - - it 'returns false for non-overlapping leaves - after' do - non_overlapping_leave = build(:leave, start_date: Date.current + 16.days, - end_date: Date.current + 20.days) - expect(base_leave.overlaps_with?(non_overlapping_leave)).to be false - end - - it 'returns false for adjacent leaves - ending where other starts' do - adjacent_leave = build(:leave, start_date: Date.current + 16.days, - end_date: Date.current + 20.days) - expect(base_leave.overlaps_with?(adjacent_leave)).to be false - end - - it 'returns false for adjacent leaves - starting where other ends' do - adjacent_leave = build(:leave, start_date: Date.current + 5.days, - end_date: Date.current + 9.days) - expect(base_leave.overlaps_with?(adjacent_leave)).to be false - end - end - - describe '#approve!' do - let(:leave) { create(:leave, status: :pending) } - - before do - create(:account_user, account: account, user: approver, role: :administrator) - end - - it 'updates status to approved' do - leave.approve!(approver) - expect(leave.status).to eq('approved') - end - - it 'sets approver' do - leave.approve!(approver) - expect(leave.approver).to eq(approver) - end - - it 'sets approved_at timestamp' do - freeze_time = Time.current - allow(Time).to receive(:current).and_return(freeze_time) - - leave.approve!(approver) - expect(leave.approved_at.to_i).to eq(freeze_time.to_i) - end - - it 'raises error if approver is not admin' do - non_admin = create(:user, account: account, role: :agent) - create(:account_user, account: account, user: non_admin, role: :agent) - - expect do - leave.approve!(non_admin) - end.to raise_error(ActiveRecord::RecordInvalid, /must be an administrator/) - end - end - - describe '#reject!' do - let(:leave) { create(:leave, status: :pending) } - - before do - create(:account_user, account: account, user: approver, role: :administrator) - end - - it 'updates status to rejected' do - leave.reject!(approver) - expect(leave.status).to eq('rejected') - end - - it 'sets approver' do - leave.reject!(approver) - expect(leave.approver).to eq(approver) - end - - it 'sets approved_at timestamp' do - freeze_time = Time.current - allow(Time).to receive(:current).and_return(freeze_time) - - leave.reject!(approver) - expect(leave.approved_at.to_i).to eq(freeze_time.to_i) - end - - it 'raises error if approver is not admin' do - non_admin = create(:user, account: account, role: :agent) - create(:account_user, account: account, user: non_admin, role: :agent) - - expect do - leave.reject!(non_admin) - end.to raise_error(ActiveRecord::RecordInvalid, /must be an administrator/) - end - end - - describe 'custom validations' do - describe '#end_date_after_start_date' do - it 'is valid when end_date is after start_date' do - leave = build(:leave, start_date: Date.current, end_date: Date.current + 1.day) - expect(leave).to be_valid - end - - it 'is valid when end_date equals start_date' do - leave = build(:leave, start_date: Date.current, end_date: Date.current) - expect(leave).to be_valid - end - - it 'is invalid when end_date is before start_date' do - leave = build(:leave, start_date: Date.current + 1.day, end_date: Date.current) - expect(leave).not_to be_valid - expect(leave.errors[:end_date]).to include('must be after start date') - end - - it 'skips validation when dates are nil' do - leave = build(:leave, start_date: nil, end_date: nil) - leave.valid? - expect(leave.errors[:end_date]).not_to include('must be after start date') - end - end - - describe '#future_dates_for_pending_leaves' do - it 'is valid for future start_date on pending leave' do - leave = build(:leave, status: :pending, start_date: Date.current + 1.day) - expect(leave).to be_valid - end - - it 'is invalid for current start_date on pending leave' do - leave = build(:leave, status: :pending, start_date: Date.current) - expect(leave).not_to be_valid - expect(leave.errors[:start_date]).to include('must be in the future') - end - - it 'is invalid for past start_date on pending leave' do - leave = build(:leave, status: :pending, start_date: Date.current - 1.day) - expect(leave).not_to be_valid - expect(leave.errors[:start_date]).to include('must be in the future') - end - - it 'skips validation for non-pending leaves' do - leave = build(:leave, status: :approved, start_date: Date.current - 1.day) - leave.valid? - expect(leave.errors[:start_date]).not_to include('must be in the future') - end - end - - describe '#approver_is_admin' do - let(:admin_user) { create(:user, account: account, role: :administrator) } - let(:agent_user) { create(:user, account: account, role: :agent) } - - before do - create(:account_user, account: account, user: admin_user, role: :administrator) - create(:account_user, account: account, user: agent_user, role: :agent) - end - - it 'is valid with admin approver' do - leave = build(:leave, account: account, approver: admin_user, status: :approved) - expect(leave).to be_valid - end - - it 'is invalid with non-admin approver' do - leave = build(:leave, account: account, approver: agent_user, status: :approved) - expect(leave).not_to be_valid - expect(leave.errors[:approved_by]).to include('must be an administrator') - end - - it 'skips validation when approver is nil' do - leave = build(:leave, account: account, approver: nil) - leave.valid? - expect(leave.errors[:approved_by]).not_to include('must be an administrator') - end - end - end -end diff --git a/spec/enterprise/policies/leave_policy_spec.rb b/spec/enterprise/policies/leave_record_policy_spec.rb similarity index 54% rename from spec/enterprise/policies/leave_policy_spec.rb rename to spec/enterprise/policies/leave_record_policy_spec.rb index 950922eac..67b17c372 100644 --- a/spec/enterprise/policies/leave_policy_spec.rb +++ b/spec/enterprise/policies/leave_record_policy_spec.rb @@ -1,6 +1,6 @@ require 'rails_helper' -RSpec.describe LeavePolicy, type: :policy do +RSpec.describe LeaveRecordPolicy, type: :policy do let(:account) { create(:account) } let(:administrator) { create(:user, account: account, role: :administrator) } let(:agent) { create(:user, account: account, role: :agent) } @@ -11,68 +11,68 @@ RSpec.describe LeavePolicy, type: :policy do let(:other_agent_context) { { user: other_agent, account: account, account_user: other_agent.account_users.find_by(account: account) } } describe 'update?' do - let(:agent_leave) { create(:leave, account: account, user: agent, status: :pending) } + let(:agent_leave_record) { create(:leave_record, account: account, user: agent, status: :pending) } - context 'when leave is pending' do + context 'when leave record is pending' do context 'when user is administrator' do - it 'allows update of any leave' do - policy = described_class.new(admin_context, agent_leave) + it 'allows update of any leave record' do + policy = described_class.new(admin_context, agent_leave_record) expect(policy.update?).to be true end end - context 'when user owns the leave' do + context 'when user owns the leave record' do it 'allows update' do - policy = described_class.new(agent_context, agent_leave) + policy = described_class.new(agent_context, agent_leave_record) expect(policy.update?).to be true end end - context 'when user does not own the leave' do + context 'when user does not own the leave record' do it 'denies update' do - policy = described_class.new(other_agent_context, agent_leave) + policy = described_class.new(other_agent_context, agent_leave_record) expect(policy.update?).to be false end end end - context 'when leave is not pending' do + context 'when leave record is not pending' do context 'when user is administrator' do - it 'denies update of approved leave' do - approved_leave = create(:leave, account: account, user: agent, status: :approved) - policy = described_class.new(admin_context, approved_leave) + it 'denies update of approved leave record' do + approved_leave_record = create(:leave_record, account: account, user: agent, status: :approved) + policy = described_class.new(admin_context, approved_leave_record) expect(policy.update?).to be false end - it 'denies update of rejected leave' do - rejected_leave = create(:leave, account: account, user: agent, status: :rejected) - policy = described_class.new(admin_context, rejected_leave) + it 'denies update of rejected leave record' do + rejected_leave_record = create(:leave_record, account: account, user: agent, status: :rejected) + policy = described_class.new(admin_context, rejected_leave_record) expect(policy.update?).to be false end - it 'denies update of cancelled leave' do - cancelled_leave = create(:leave, account: account, user: agent, status: :cancelled) - policy = described_class.new(admin_context, cancelled_leave) + it 'denies update of cancelled leave record' do + cancelled_leave_record = create(:leave_record, account: account, user: agent, status: :cancelled) + policy = described_class.new(admin_context, cancelled_leave_record) expect(policy.update?).to be false end end - context 'when user owns the leave' do - it 'denies update of approved leave' do - approved_leave = create(:leave, account: account, user: agent, status: :approved) - policy = described_class.new(agent_context, approved_leave) + context 'when user owns the leave record' do + it 'denies update of approved leave record' do + approved_leave_record = create(:leave_record, account: account, user: agent, status: :approved) + policy = described_class.new(agent_context, approved_leave_record) expect(policy.update?).to be false end - it 'denies update of rejected leave' do - rejected_leave = create(:leave, account: account, user: agent, status: :rejected) - policy = described_class.new(agent_context, rejected_leave) + it 'denies update of rejected leave record' do + rejected_leave_record = create(:leave_record, account: account, user: agent, status: :rejected) + policy = described_class.new(agent_context, rejected_leave_record) expect(policy.update?).to be false end - it 'denies update of cancelled leave' do - cancelled_leave = create(:leave, account: account, user: agent, status: :cancelled) - policy = described_class.new(agent_context, cancelled_leave) + it 'denies update of cancelled leave record' do + cancelled_leave_record = create(:leave_record, account: account, user: agent, status: :cancelled) + policy = described_class.new(agent_context, cancelled_leave_record) expect(policy.update?).to be false end end @@ -80,95 +80,95 @@ RSpec.describe LeavePolicy, type: :policy do end describe 'destroy?' do - context 'when leave can be cancelled' do + context 'when leave record can be cancelled' do context 'when user is administrator' do - it 'allows destroy of pending leave' do - pending_leave = create(:leave, account: account, user: agent, status: :pending) - policy = described_class.new(admin_context, pending_leave) + it 'allows destroy of pending leave record' do + pending_leave_record = create(:leave_record, account: account, user: agent, status: :pending) + policy = described_class.new(admin_context, pending_leave_record) expect(policy.destroy?).to be true end - it 'allows destroy of approved future leave' do - approved_future_leave = create(:leave, account: account, user: agent, status: :approved, + it 'allows destroy of approved future leave record' do + approved_future_leave_record = create(:leave_record, account: account, user: agent, status: :approved, start_date: 1.week.from_now.to_date, end_date: 2.weeks.from_now.to_date) - policy = described_class.new(admin_context, approved_future_leave) + policy = described_class.new(admin_context, approved_future_leave_record) expect(policy.destroy?).to be true end - it 'allows destroy of other user leaves' do - other_leave = create(:leave, account: account, user: other_agent, status: :pending) - policy = described_class.new(admin_context, other_leave) + it 'allows destroy of other user leave records' do + other_leave_record = create(:leave_record, account: account, user: other_agent, status: :pending) + policy = described_class.new(admin_context, other_leave_record) expect(policy.destroy?).to be true end end - context 'when user owns the leave' do - it 'allows destroy of own pending leave' do - pending_leave = create(:leave, account: account, user: agent, status: :pending) - policy = described_class.new(agent_context, pending_leave) + context 'when user owns the leave record' do + it 'allows destroy of own pending leave record' do + pending_leave_record = create(:leave_record, account: account, user: agent, status: :pending) + policy = described_class.new(agent_context, pending_leave_record) expect(policy.destroy?).to be true end - it 'allows destroy of own approved future leave' do - approved_future_leave = create(:leave, account: account, user: agent, status: :approved, + it 'allows destroy of own approved future leave record' do + approved_future_leave_record = create(:leave_record, account: account, user: agent, status: :approved, start_date: 1.week.from_now.to_date, end_date: 2.weeks.from_now.to_date) - policy = described_class.new(agent_context, approved_future_leave) + policy = described_class.new(agent_context, approved_future_leave_record) expect(policy.destroy?).to be true end end - context 'when user does not own the leave' do - it 'denies destroy of other user leave' do - pending_leave = create(:leave, account: account, user: agent, status: :pending) - policy = described_class.new(other_agent_context, pending_leave) + context 'when user does not own the leave record' do + it 'denies destroy of other user leave record' do + pending_leave_record = create(:leave_record, account: account, user: agent, status: :pending) + policy = described_class.new(other_agent_context, pending_leave_record) expect(policy.destroy?).to be false end end end - context 'when leave cannot be cancelled' do + context 'when leave record cannot be cancelled' do context 'when user is administrator' do - it 'denies destroy of current approved leave' do - approved_current_leave = create(:leave, account: account, user: agent, status: :approved, + it 'denies destroy of current approved leave record' do + approved_current_leave_record = create(:leave_record, account: account, user: agent, status: :approved, start_date: Date.current, end_date: Date.current + 2.days) - policy = described_class.new(admin_context, approved_current_leave) + policy = described_class.new(admin_context, approved_current_leave_record) expect(policy.destroy?).to be false end - it 'denies destroy of past approved leave' do - approved_past_leave = create(:leave, account: account, user: agent, status: :approved, + it 'denies destroy of past approved leave record' do + approved_past_leave_record = create(:leave_record, account: account, user: agent, status: :approved, start_date: 1.week.ago.to_date, end_date: 3.days.ago.to_date) - policy = described_class.new(admin_context, approved_past_leave) + policy = described_class.new(admin_context, approved_past_leave_record) expect(policy.destroy?).to be false end - it 'denies destroy of rejected leave' do - rejected_leave = create(:leave, account: account, user: agent, status: :rejected) - policy = described_class.new(admin_context, rejected_leave) + it 'denies destroy of rejected leave record' do + rejected_leave_record = create(:leave_record, account: account, user: agent, status: :rejected) + policy = described_class.new(admin_context, rejected_leave_record) expect(policy.destroy?).to be false end end - context 'when user owns the leave' do - it 'denies destroy of current approved leave' do - approved_current_leave = create(:leave, account: account, user: agent, status: :approved, + context 'when user owns the leave record' do + it 'denies destroy of current approved leave record' do + approved_current_leave_record = create(:leave_record, account: account, user: agent, status: :approved, start_date: Date.current, end_date: Date.current + 2.days) - policy = described_class.new(agent_context, approved_current_leave) + policy = described_class.new(agent_context, approved_current_leave_record) expect(policy.destroy?).to be false end - it 'denies destroy of past approved leave' do - approved_past_leave = create(:leave, account: account, user: agent, status: :approved, + it 'denies destroy of past approved leave record' do + approved_past_leave_record = create(:leave_record, account: account, user: agent, status: :approved, start_date: 1.week.ago.to_date, end_date: 3.days.ago.to_date) - policy = described_class.new(agent_context, approved_past_leave) + policy = described_class.new(agent_context, approved_past_leave_record) expect(policy.destroy?).to be false end - it 'denies destroy of rejected leave' do - rejected_leave = create(:leave, account: account, user: agent, status: :rejected) - policy = described_class.new(agent_context, rejected_leave) + it 'denies destroy of rejected leave record' do + rejected_leave_record = create(:leave_record, account: account, user: agent, status: :rejected) + policy = described_class.new(agent_context, rejected_leave_record) expect(policy.destroy?).to be false end end @@ -177,14 +177,14 @@ RSpec.describe LeavePolicy, type: :policy do describe 'Scope' do before do - create(:leave, account: account, user: administrator) - create(:leave, account: account, user: agent) - create(:leave, account: account, user: other_agent) + create(:leave_record, account: account, user: administrator) + create(:leave_record, account: account, user: agent) + create(:leave_record, account: account, user: other_agent) end context 'when user is administrator' do - it 'returns all leaves in account with includes' do - scope = LeavePolicy::Scope.new(admin_context, Leave.all) + it 'returns all leave records in account with includes' do + scope = LeaveRecordPolicy::Scope.new(admin_context, account.leave_records) result = scope.resolve expect(result.count).to eq(3) @@ -194,8 +194,8 @@ RSpec.describe LeavePolicy, type: :policy do end context 'when user is not administrator' do - it 'returns only own leaves with includes' do - scope = LeavePolicy::Scope.new(agent_context, Leave.all) + it 'returns only own leave records with includes' do + scope = LeaveRecordPolicy::Scope.new(agent_context, account.leave_records) result = scope.resolve expect(result.count).to eq(1) @@ -204,7 +204,7 @@ RSpec.describe LeavePolicy, type: :policy do end it 'filters correctly for other agent' do - scope = LeavePolicy::Scope.new(other_agent_context, Leave.all) + scope = LeaveRecordPolicy::Scope.new(other_agent_context, account.leave_records) result = scope.resolve expect(result.count).to eq(1) @@ -214,28 +214,28 @@ RSpec.describe LeavePolicy, type: :policy do context 'when initializing scope' do it 'properly initializes all context variables' do - scope = LeavePolicy::Scope.new(admin_context, Leave.all) + scope = LeaveRecordPolicy::Scope.new(admin_context, account.leave_records) expect(scope.user_context).to eq(admin_context) expect(scope.user).to eq(administrator) expect(scope.account).to eq(account) expect(scope.account_user).to eq(administrator.account_users.find_by(account: account)) - expect(scope.scope).to eq(Leave.all) + expect(scope.scope).to eq(account.leave_records) end end end describe 'ownership checks' do describe '#owned_by_user?' do - it 'returns true when user owns the leave' do - agent_leave = create(:leave, account: account, user: agent, status: :pending) - policy = described_class.new(agent_context, agent_leave) + it 'returns true when user owns the leave record' do + agent_leave_record = create(:leave_record, account: account, user: agent, status: :pending) + policy = described_class.new(agent_context, agent_leave_record) expect(policy.send(:owned_by_user?)).to be true end - it 'returns false when user does not own the leave' do - agent_leave = create(:leave, account: account, user: agent, status: :pending) - policy = described_class.new(other_agent_context, agent_leave) + it 'returns false when user does not own the leave record' do + agent_leave_record = create(:leave_record, account: account, user: agent, status: :pending) + policy = described_class.new(other_agent_context, agent_leave_record) expect(policy.send(:owned_by_user?)).to be false end @@ -245,4 +245,4 @@ RSpec.describe LeavePolicy, type: :policy do end end end -end +end \ No newline at end of file diff --git a/spec/factories/leaves.rb b/spec/factories/leave_records.rb similarity index 89% rename from spec/factories/leaves.rb rename to spec/factories/leave_records.rb index 7b63873b8..082c50c11 100644 --- a/spec/factories/leaves.rb +++ b/spec/factories/leave_records.rb @@ -1,5 +1,5 @@ FactoryBot.define do - factory :leave do + factory :leave_record do account user start_date { 1.week.from_now.to_date } @@ -38,6 +38,7 @@ FactoryBot.define do trait :past_dates do start_date { 2.weeks.ago.to_date } end_date { 1.week.ago.to_date } + status { :approved } # Past dates should only be used with approved status end end end