From 0fe9a3a5e001717958844f59df6580a9130ef77f Mon Sep 17 00:00:00 2001 From: Tanmay Deep Sharma Date: Wed, 20 Aug 2025 14:49:25 +0530 Subject: [PATCH] self review changes --- config/locales/en.yml | 3 +++ db/migrate/20250806140005_create_leaves.rb | 2 -- ...315_rename_leave_table_to_leave_records.rb | 5 ++++ db/schema.rb | 17 ++++++++----- .../v1/accounts/leave_records_controller.rb | 24 ++++++++++--------- .../app/models/enterprise/concerns/account.rb | 2 +- .../app/models/enterprise/concerns/user.rb | 2 +- enterprise/app/models/leave_record.rb | 12 ++++------ 8 files changed, 39 insertions(+), 28 deletions(-) create mode 100644 db/migrate/20250820083315_rename_leave_table_to_leave_records.rb diff --git a/config/locales/en.yml b/config/locales/en.yml index 1d8347679..c1f977549 100644 --- a/config/locales/en.yml +++ b/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 diff --git a/db/migrate/20250806140005_create_leaves.rb b/db/migrate/20250806140005_create_leaves.rb index 982ec1181..acf66949b 100644 --- a/db/migrate/20250806140005_create_leaves.rb +++ b/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| diff --git a/db/migrate/20250820083315_rename_leave_table_to_leave_records.rb b/db/migrate/20250820083315_rename_leave_table_to_leave_records.rb new file mode 100644 index 000000000..2436a11f0 --- /dev/null +++ b/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 diff --git a/db/schema.rb b/db/schema.rb index 7f44c373c..3d62e5416 100644 --- a/db/schema.rb +++ b/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| diff --git a/enterprise/app/controllers/api/v1/accounts/leave_records_controller.rb b/enterprise/app/controllers/api/v1/accounts/leave_records_controller.rb index a65b03172..b2e724516 100644 --- a/enterprise/app/controllers/api/v1/accounts/leave_records_controller.rb +++ b/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 diff --git a/enterprise/app/models/enterprise/concerns/account.rb b/enterprise/app/models/enterprise/concerns/account.rb index e29803645..7ef192578 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 :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' diff --git a/enterprise/app/models/enterprise/concerns/user.rb b/enterprise/app/models/enterprise/concerns/user.rb index 4c8d3761c..1721ee315 100644 --- a/enterprise/app/models/enterprise/concerns/user.rb +++ b/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 diff --git a/enterprise/app/models/leave_record.rb b/enterprise/app/models/leave_record.rb index f2d95c2f6..efb83eeaf 100644 --- a/enterprise/app/models/leave_record.rb +++ b/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