self review changes

config/locales/en.yml

@@ -96,6 +96,9 @@ en:
       invalid_value: Invalid value. The values provided for %{attribute_name} are invalid
     custom_attribute_definition:
       key_conflict: The provided key is not allowed as it might conflict with default attributes.
+    leave_records:
+      cannot_update_non_pending: Cannot update non-pending leave record
+      cannot_delete: Cannot delete this leave record
   reports:
     period: Reporting period %{since} to %{until}
     utc_warning: The report generated is in UTC timezone

db/migrate/20250806140005_create_leaves.rb

@@ -1,5 +1,3 @@
-# frozen_string_literal: true
-
 class CreateLeaves < ActiveRecord::Migration[7.1]
   def change
     create_table :leaves do |t|

db/migrate/20250820083315_rename_leave_table_to_leave_records.rb

@@ -0,0 +1,5 @@
+class RenameLeaveTableToLeaveRecords < ActiveRecord::Migration[7.1]
+  def change
+    rename_table :leaves, :leave_records
+  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[7.1].define(version: 2025_08_08_123008) do
+ActiveRecord::Schema[7.1].define(version: 2025_08_20_083315) do
   # These extensions should be enabled to support this database
   enable_extension "pg_stat_statements"
   enable_extension "pg_trgm"
@@ -320,9 +320,14 @@ ActiveRecord::Schema[7.1].define(version: 2025_08_08_123008) do
     t.datetime "created_at", null: false
     t.datetime "updated_at", null: false
     t.integer "status", default: 0, null: false
+    t.string "content_type"
+    t.bigint "file_size"
+    t.jsonb "metadata", default: {}
     t.index ["account_id"], name: "index_captain_documents_on_account_id"
     t.index ["assistant_id", "external_link"], name: "index_captain_documents_on_assistant_id_and_external_link", unique: true
     t.index ["assistant_id"], name: "index_captain_documents_on_assistant_id"
+    t.index ["content_type"], name: "index_captain_documents_on_content_type"
+    t.index ["metadata"], name: "index_captain_documents_on_metadata", using: :gin
     t.index ["status"], name: "index_captain_documents_on_status"
   end
 
@@ -848,7 +853,7 @@ ActiveRecord::Schema[7.1].define(version: 2025_08_08_123008) do
     t.index ["title", "account_id"], name: "index_labels_on_title_and_account_id", unique: true
   end
 
-  create_table "leaves", force: :cascade do |t|
+  create_table "leave_records", force: :cascade do |t|
     t.bigint "account_id", null: false
     t.bigint "user_id", null: false
     t.date "start_date", null: false
@@ -860,10 +865,10 @@ ActiveRecord::Schema[7.1].define(version: 2025_08_08_123008) do
     t.datetime "approved_at"
     t.datetime "created_at", null: false
     t.datetime "updated_at", null: false
-    t.index ["account_id", "status"], name: "index_leaves_on_account_id_and_status"
-    t.index ["account_id"], name: "index_leaves_on_account_id"
-    t.index ["approved_by_id"], name: "index_leaves_on_approved_by_id"
-    t.index ["user_id"], name: "index_leaves_on_user_id"
+    t.index ["account_id", "status"], name: "index_leave_records_on_account_id_and_status"
+    t.index ["account_id"], name: "index_leave_records_on_account_id"
+    t.index ["approved_by_id"], name: "index_leave_records_on_approved_by_id"
+    t.index ["user_id"], name: "index_leave_records_on_user_id"
   end
 
   create_table "macros", force: :cascade do |t|

enterprise/app/controllers/api/v1/accounts/leave_records_controller.rb

@@ -1,6 +1,8 @@
 class Api::V1::Accounts::LeaveRecordsController < Api::V1::Accounts::EnterpriseAccountsController
   before_action :fetch_leave_record, only: [:show, :update, :destroy, :approve, :reject]
   before_action :check_authorization
+  before_action :ensure_pending_status, only: [:update]
+  before_action :ensure_can_be_cancelled, only: [:destroy]
 
   def index
     @leave_records = policy_scope(Current.account.leave_records)
@@ -13,14 +15,10 @@ class Api::V1::Accounts::LeaveRecordsController < Api::V1::Accounts::EnterpriseA
   end
 
   def update
-    render json: { error: 'Cannot update non-pending leave record' }, status: :unprocessable_entity and return unless @leave_record.pending?
-
     @leave_record.update!(permitted_params)
   end
 
   def destroy
-    render json: { error: 'Cannot delete this leave record' }, status: :unprocessable_entity and return unless @leave_record.can_be_cancelled?
-
     @leave_record.destroy!
     head :ok
   end
@@ -42,14 +40,18 @@ class Api::V1::Accounts::LeaveRecordsController < Api::V1::Accounts::EnterpriseA
   end
 
   def fetch_leave_record
-    @leave_record = leave_records_scope.find(params[:id])
+    @leave_record = policy_scope(Current.account.leave_records).find(params[:id])
   end
 
-  def leave_records_scope
-    if Current.account_user.administrator?
-      Current.account.leave_records
-    else
-      Current.account.leave_records.where(user: current_user)
-    end
+  def ensure_pending_status
+    return if @leave_record.pending?
+
+    render_could_not_create_error(I18n.t('errors.leave_records.cannot_update_non_pending'))
+  end
+
+  def ensure_can_be_cancelled
+    return if @leave_record.can_be_cancelled?
+
+    render_could_not_create_error(I18n.t('errors.leave_records.cannot_delete'))
   end
 end

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 :leave_records, dependent: :destroy_async, class_name: 'LeaveRecord'
+    has_many :leave_records, dependent: :destroy_async
 
     has_many :captain_assistants, dependent: :destroy_async, class_name: 'Captain::Assistant'
     has_many :captain_assistant_responses, dependent: :destroy_async, class_name: 'Captain::AssistantResponse'

enterprise/app/models/enterprise/concerns/user.rb

@@ -6,7 +6,7 @@ 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 :leave_records, dependent: :destroy_async, class_name: 'LeaveRecord'
+    has_many :leave_records, dependent: :destroy_async
     has_many :approved_leave_records, class_name: 'LeaveRecord', foreign_key: 'approved_by_id', dependent: :nullify, inverse_of: :approver
   end
 

enterprise/app/models/leave_record.rb

@@ -1,6 +1,6 @@
 # == Schema Information
 #
-# Table name: leaves
+# Table name: leave_records
 #
 #  id             :bigint           not null, primary key
 #  approved_at    :datetime
@@ -17,14 +17,12 @@
 #
 # Indexes
 #
-#  index_leaves_on_account_id             (account_id)
-#  index_leaves_on_account_id_and_status  (account_id,status)
-#  index_leaves_on_approved_by_id         (approved_by_id)
-#  index_leaves_on_user_id                (user_id)
+#  index_leave_records_on_account_id             (account_id)
+#  index_leave_records_on_account_id_and_status  (account_id,status)
+#  index_leave_records_on_approved_by_id         (approved_by_id)
+#  index_leave_records_on_user_id                (user_id)
 #
 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_leave_records